Re: [flashrom] [PATCH] Use strlen for const strings

[David Borg]

Don't know whether I can ack this, but after modifiying the conditionals to
always compile that part of the code, gcc did in fact compute the strlen
result.

$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.4.4-14ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.4 --enable-shared --enable-multiarch
--enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib --enable-nls
--with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug
--enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=x86_64-linux-gnu
Thread model: posix
gcc version 4.4.5 (Ubuntu/Linaro 4.4.4-14ubuntu5)

Relevant outputs from processor_enable.c.003t.original after adding
-fdump-tree-original to CFLAGS in the Makefile are inlined:

On 30 September 2010 02:41, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> Replace sizeof("string")-1 with strlen("string")
>
> We want to avoid calls to strlen at runtime if the string is already
> known at compile time.
> Turns out that gcc and clang will recognize constant strings and compute
> the strlen result already at compile time, so trickery with sizeof only
> reduces readability but does not improve the code.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-const_strlen/processor_enable.c
>
> --- flashrom-const_strlen/processor_enable.c    (Revision 1183)
> +++ flashrom-const_strlen/processor_enable.c    (Arbeitskopie)
> @@ -56,13 +56,13 @@
>                while (*ptr && isspace(*ptr))
>                        ptr++;
>                /* "cpu" part appears only with some Linux versions.  */
>           if (strncmp(ptr, "cpu", sizeof("cpu") - 1) == 0)
>                   ptr += sizeof("cpu") - 1;
>           if (strncmp(ptr, "cpu", strlen("cpu")) == 0)
>                   ptr += strlen("cpu");

if (strncmp ((const char *) ptr, (const char *) "cpu", 3) == 0)
      {
        ptr = ptr + 3;
      }
>            while (*ptr && isspace(*ptr))
>                    ptr++;
>           if (strncmp(ptr, "model", sizeof("model") - 1) != 0)
>           if (strncmp(ptr, "model", strlen("model")) != 0)
>                  continue;
>           ptr += sizeof("model") - 1;
>           ptr += strlen("model");

if (strncmp ((const char *) ptr, (const char *) "model", 5) != 0)
      {
        // predicted unlikely by continue predictor.;
        goto <D.4030>;
      }
    ptr = ptr + 5;
>            while (*ptr && isspace(*ptr))
>                    ptr++;
>            if (*ptr != ':')
>
> @@ -72,9 +72,9 @@
>                        ptr++;
>                fclose(cpuinfo);
>                return (strncmp(ptr, "ICT Loongson-2 V0.3",
>                           sizeof("ICT Loongson-2 V0.3") - 1) == 0)
>                           strlen("ICT Loongson-2 V0.3")) == 0)
>              || (strncmp(ptr, "Godson2 V0.3  FPU V0.1",
>                           sizeof("Godson2 V0.3  FPU V0.1") - 1) ==
>
> 0);
>                           strlen("Godson2 V0.3  FPU V0.1")) == 0);
>  }

fclose (cpuinfo);
    return strncmp ((const char *) ptr, (const char *) "ICT Loongson-2
V0.3", 19) == 0 || strncmp ((const char *) ptr, (const char *) "Godson2
V0.3  FPU V0.1", 22) == $
  }
>    fclose(cpuinfo);
>    return 0;
>
>
> flashrom mailing list
> [e-mail]
> http://www.flashrom.org/mailman/listinfo/flashrom