On Tue, Jun 07, 2011 at 06:27:27PM +0200, Jim Meyering wrote: > H. Peter Anvin wrote: > > On 06/07/2011 08:53 AM, Jim Meyering wrote: > >> Yes, just like the original code. > >> The reason I prefer not to ignore the return value is to avoid > >> warnings from static analyzers. > > > > Declare it (void) explicitly then... > > With some versions of gcc, the (void) cast does not suffice > to avoid the warn-unused-result warnings. > That's why the gnulib "ignore-value" module came to be. > So we can do this: > > ignore_value (sscanf (...)); > > But that (like the void cast) is pretty ugly. > But using two sscanf calls with the associated logic is even uglier, > so I'll go for your change after all. > > However, I don't want to ignore the return value. > > How about the patch below? > I left your name on it. > Let me know if that's ok. > > >From 0b6cd440ac094cefd791dba9d19d112aafd09fdf Mon Sep 17 00:00:00 2001 > From: "H. Peter Anvin" <[email protected]> > Date: Tue, 7 Jun 2011 18:03:14 +0200 > Subject: [PATCH] libparted: simplify kernel version-parsing logic > > * libparted/arch/linux.c: (_get_linux_version): Simplify and add > an assertion. > --- > libparted/arch/linux.c | 15 +++++---------- > 1 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c > index 111816c..bfe2982 100644 > --- a/libparted/arch/linux.c > +++ b/libparted/arch/linux.c > @@ -601,22 +601,17 @@ _get_linux_version () > static int kver = -1; > > struct utsname uts; > - int major; > - int minor; > - int teeny; > + int major = 0; > + int minor = 0; > + int teeny = 0; > > if (kver != -1) > return kver; > > if (uname (&uts)) > return kver = 0; > - if (sscanf (uts.release, "%u.%u.%u", &major, &minor, &teeny) == 3) > - ; /* ok */ > - else if (sscanf (uts.release, "%u.%u", &major, &minor) == 2) > - teeny = 0; > - else > - return kver = 0; > - > + int n = sscanf (uts.release, "%u.%u.%u", &major, &minor, &teeny); > + assert (n == 2 || n == 3); > return kver = KERNEL_VERSION (major, minor, teeny); > } > > -- > 1.7.6.rc0.293.g40857
The patch looks good (I didn't test it). It's definitely an improvement that the code doesn't return 0 when it doesn't know how to parse the release string. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora _______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/parted-devel
