Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dist/ethos: fixed compile warnings #5317

Merged
merged 1 commit into from
Apr 14, 2016

Conversation

haukepetersen
Copy link
Contributor

  • checking return values of each write() call
  • chack return value of system() call

@haukepetersen haukepetersen added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Apr 14, 2016
@haukepetersen haukepetersen added this to the Release 2016.04 milestone Apr 14, 2016
static void checked_write(int handle, void *buffer, int nbyte)
{
if (write(handle, buffer, nbyte) != nbyte) {
fprintf(stderr, "write to fd %i failed\n", handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fprintf(stderr, "write to fd %i failed: %s\n", handle, strerror(errno)));

@kaspar030
Copy link
Contributor

Much better. I you feel like, add the actual error message to the output (see other comment). ACK.

@haukepetersen
Copy link
Contributor Author

fixed write error message and squashed.

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 14, 2016
@haukepetersen
Copy link
Contributor Author

just came to mind when setting the 'read for...' label: tools like ethos are not checked by the CI in any case, right? Does it make sense to include them (open an issue for that)? I think it might not be too bad to do this...

@kaspar030 kaspar030 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 14, 2016
@kaspar030
Copy link
Contributor

a ")" too much:

[kaspar@booze ethos (test)]$ make
cc -O3 -Wall ethos.c -o ethos
ethos.c: In function ‘checked_write’:
ethos.c:39:80: error: expected ‘;’ before ‘)’ token
         fprintf(stderr, "write to fd %i failed: %s\n", handle, strerror(errno)));
                                                                                ^
ethos.c:39:80: error: expected statement before ‘)’ token
Makefile:4: recipe for target 'ethos' failed
make: *** [ethos] Error 1
[kaspar@booze ethos (test)]$

@kaspar030
Copy link
Contributor

tools like ethos are not checked by the CI in any case, right? Does it make sense to include them (open an issue for that)? I think it might not be too bad to do this...

yes and yes...

@haukepetersen
Copy link
Contributor Author

done #5319

@haukepetersen
Copy link
Contributor Author

so are you bold enough to just merge this?

@kaspar030
Copy link
Contributor

Did you fix the bracket?

@kaspar030
Copy link
Contributor

haukepetersen#27

@haukepetersen
Copy link
Contributor Author

nope, github did not update the view before my comments, so I did not see your input.

- checking return values of each write() call
- chack return value of system() call
@haukepetersen
Copy link
Contributor Author

squashed

@kaspar030
Copy link
Contributor

compiled & tested here, works like a charm. skipping murdock as this doesn't get compiled anywasy. go!

@kaspar030 kaspar030 merged commit e6d90e3 into RIOT-OS:master Apr 14, 2016
@kaspar030
Copy link
Contributor

Thanks @haukepetersen for taking the time to clean up my sloppyness!

@haukepetersen
Copy link
Contributor Author

my pleasure :-)

@haukepetersen haukepetersen deleted the fix_ethos_compileerrors branch April 14, 2016 16:53
@cgundogan
Copy link
Member

does this need backporting?

@haukepetersen
Copy link
Contributor Author

yes

@haukepetersen haukepetersen added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 18, 2016
@cgundogan cgundogan removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Apr 18, 2016
miri64 added a commit that referenced this pull request Apr 19, 2016
…errors

dist/ethos: fixed compile warnings  (backport of #5317)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants