GNOME Bugzilla – Bug 657129
check the return values of I/O calls
Last modified: 2018-05-24 13:01:47 UTC
Created attachment 194442 [details] [review] A patch to fix the problem If a signal comes before the write happens, the write fails and returns -1 with errno set to EINTR. If a signal comes after the write started, the write will return the numbers of bytes written. I'm adding a patch that makes the write restart in the first case and continue in the second.
Thanks. But wouldn't it be better to use sigaction and SA_RESTART so we don't need this change all over the place?
Ok you want to fix warnings, then SA_RESTART won't do I guess. In that case, you should add a helper function, let's call it gimp_write_retry() or something, that we can reuse.
First, we use sigaction and SA_RESTART already, second, what warnings do you mean Martin, third, temp_buf_dump() is comletely unused, let's rather remove it.
The warning is: make[4]: Entering directory `/usr/local/gimp/app/base' CC temp-buf.o temp-buf.c: In function ‘temp_buf_dump’: temp-buf.c:403:9: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result] A write will restart if it hasn't transferred any data yet, but if some data has been transferred then even with SA_RESTART the write will return and tell you the number of bytes that have been transferred. I suppose I should get the source of the filesystem and really check. signal(7) says that slow I/O (not disk) will restart, but doesn't really say what will happen to disk I/O. Is it guaranteed to always finish absent running out of space? Of course then you should still check the return code. I agree about removing the method if unused. There are other routines that don't check the return values of things that should be checked though.
Which other routines?
Please tell us which other routines are affected.
Patrick, can you please provide requested information ?
Created attachment 208376 [details] List of places in gimp source that return values are not checked. Sorry it took so long to provide. It's a list of all the rest of the places.
Thanks, that looks good :) Now we need somebody to look at all these places...
Comment on attachment 194442 [details] [review] A patch to fix the problem This was a patch for unused and obsolete debug code.
In master, app/ got completely ported to GIO, with all calls being error-checked, so this only affects plug-ins. They should be ported to GIO too.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/374.