After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 698348 - glocalfile.c build fix
glocalfile.c build fix
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.36.x
Other NetBSD
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 697166 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-04-19 09:30 UTC by Patrick Welche
Modified: 2014-01-20 06:08 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
glocalfile statvfs compile fix (823 bytes, patch)
2013-04-19 09:34 UTC, Patrick Welche 		needs-work Details | Review
repeated declaration in #ifdef USE_STATVFS section (699 bytes, patch)
2013-05-31 22:31 UTC, Patrick Welche 		none Details | Review

Description Patrick Welche 2013-04-19 09:30:34 UTC 
In is_remote_fs(), statfs_result is also used in the USE_STATVFS case.
(I was tempted to s/statfs_result/retval/ but didn't - would that be preferred?)
Comment 1 Patrick Welche 2013-04-19 09:34:11 UTC 
Created attachment 241885 [details] [review]
glocalfile statvfs compile fix
Comment 2 Patrick Welche 2013-05-31 12:50:15 UTC 
ping? Surely this is trivially correct and can be applied!
Comment 3 Allison Karlitskaya (desrt) 2013-05-31 13:38:58 UTC 
Review of attachment 241885 [details] [review]:

Sorry for ignoring this -- it fell through the cracks.

Your fix isn't _quite_ right because then this would be an unused variable in the #else case (not USE_STATFS and not USE_STATVFS).  I think the correct fix is rather to copy the variable declaration down to under the #elif below.  Feel free to rename it if you like.
Comment 4 Matthias Clasen 2013-05-31 13:51:09 UTC 
we've been dealing with the spaghetti ifdefs in that file for quite a while now - maybe we should clean that up for real...
Comment 5 Allison Karlitskaya (desrt) 2013-05-31 14:01:58 UTC 
That code is new -- it was for our detect-NFS-and-use-FAM thing.  Unfortunately the spaghetti is required.  Best we could do, really, is split it into some different functions.
Comment 6 Dan Winship 2013-05-31 14:25:30 UTC 
I think the fact that Patrick's fix contained another bug is a pretty good sign that the spaghetti has won, and we need to find a better way to organize this code.
Comment 7 Patrick Welche 2013-05-31 22:27:47 UTC 
Actually, it isn't that obvious that it contains another bug: outside all #ifdefs, there is a "if (statfs_result == -1)", so I thought that declaring statfs_result outside of all #ifdefs was a good idea as it is referenced outside of all #ifdefs. If you think there will be an unused variable warning, you have to take into account the "return" before that line is reached... (Either way, the warning is better than the error)

Agreed that the spaghetti is getting fiddly, but in glocalfile.c it is only that one function is_remote_fs which is tricky (as opposed to gunixmounts.c which could probably do with being split into separate files...)
Comment 8 Patrick Welche 2013-05-31 22:31:25 UTC 
Created attachment 245784 [details] [review]
repeated declaration in #ifdef USE_STATVFS section

This version duplicates the declaration as suggested. I didn't rename the variable in the end as it all has a certain symmetry.
Comment 9 Fabian Groffen 2013-09-13 19:12:59 UTC 
Same problem also on Solaris, same patch works fine.  Please apply a fix for the next version.
Comment 10 Patrick Welche 2013-09-14 23:36:11 UTC 
It seems that my patch was rediscovered 3 months later by Igor in Bug 704587. He was luckier in getting it applied...
Comment 11 Matthias Clasen 2014-01-20 06:08:50 UTC 
*** Bug 697166 has been marked as a duplicate of this bug. ***