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 726653 - fixes/improvements to ifcfg-rh shvar code
fixes/improvements to ifcfg-rh shvar code
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: Distro-specific
0.9.8
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-03-18 17:20 UTC by Dan Winship
Modified: 2014-07-10 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Dan Winship 2014-03-18 17:20:08 UTC
While looking at rh#1070617, I decided that svNewFile() should return a GError, so that when we failed to read a file, we could log the exact reason why. Then I realized that shvar.c was a complete disaster.

See danw/shvar for fixes. (For comparison, the first commit in the branch is what I proposed for the downstream bug, though I guess there's no reason to commit it upstream since it gets undone by the last commit.)
Comment 1 Thomas Haller 2014-03-18 18:56:59 UTC
> ifcfg-rh: syntactic code style fixes to shvar.[ch]
> ifcfg-rh: rename svNewFile() to svOpenFile()

Could you mention "trivial" in both commit messages? After looking at it, the commits seem really not to make any semantic changes whatsoever.


> ifcfg-rh: clean up lowlevel ifcfg file reading code

Make the EINTR line a separate patch that we might backport?



Rest looks good!!


Btw. could we also fixup the tab vs. whitespace one day? It is quite annoying to look at those files -- of course it's also annoying to do any backports later on :(
Comment 2 Dan Winship 2014-03-18 19:28:34 UTC
(In reply to comment #1)
> Btw. could we also fixup the tab vs. whitespace one day? It is quite annoying
> to look at those files -- of course it's also annoying to do any backports
> later on :(

The "syntactic" patch was supposed to completely fix up the whitespace in this file. Was there anything left wrong? Or did you mean fix it across the whole tree?
Comment 3 Thomas Haller 2014-03-18 19:30:47 UTC
Ah ok. I didn't notice, because I watched it with
  git log -p --color-words=.

Great then!!
Comment 4 Dan Winship 2014-03-20 14:59:13 UTC
(In reply to comment #1)
> > ifcfg-rh: clean up lowlevel ifcfg file reading code
> 
> Make the EINTR line a separate patch that we might backport?

Well... there's no real reason to not backport the s->arena part too though; it's simple and obviously correct. I did change the commit message to emphasize the EINTR part more though, since that's actually a correctness fix.

Re-pushed, with the commits reordered so that the possibly-backportable ones all come before the syntactic/semantic rewrites now. (This includes splitting out the g_ascii_strcasecmp() fix into its own commit.)
Comment 5 Dan Winship 2014-04-09 14:54:19 UTC
pushed
Comment 6 Allison Karlitskaya (desrt) 2014-04-09 16:15:26 UTC
I just hit this in jhbuild:

make[7]: Entering directory `/home/desrt/.cache/jhbuild/checkout/NetworkManager/src/settings/plugins/ifcfg-rh'
  CC       writer.lo
writer.c: In function 'write_ip4_aliases':
writer.c:2230:3: error: too few arguments to function 'svWriteFile' svWriteFile (ifcfg, 0644);
Comment 7 Dan Winship 2014-04-09 17:32:46 UTC
fixed now. (i didn't try rebuilding after rebasing to master, and there was new code that needed updating)
Comment 8 Dan Winship 2014-07-10 13:12:56 UTC
this was fixed but never got closed...