[parted-devel] Problem with Linux "3.0"

[Richard W.M. Jones]

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" <hpa at zytor.com>
> 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