[PATCH] Use strlen for const strings

[Carl-Daniel Hailfinger]

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");
    	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");
    	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 0;

[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

[Sean Nelson]

On 9/29/10 5:41 PM, Carl-Daniel Hailfinger 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

Acked-by: Sean Nelson

[David Borg]

Carl-Daniel, please run 'svn up' in the source directory before committing,
as the patch won't apply directly as-is. svn will handle the merge
correctly.
Thanks!

On 15 October 2010 01:47, Sean Nelson  wrote:
> On 9/29/10 5:41 PM, Carl-Daniel Hailfinger 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
>
> Acked-by: Sean Nelson
>
> flashrom mailing list
> [e-mail]
> http://www.flashrom.org/mailman/listinfo/flashrom

[Uwe Hermann]

On Fri, Oct 15, 2010 at 02:06:16PM +0200, David Borg wrote:
> Carl-Daniel, please run 'svn up' in the source directory before committing,
> as the patch won't apply directly as-is. svn will handle the merge
> correctly.
> Thanks!

Updated patch attached. It's also
Acked-by: Uwe Hermann

Uwe.

[Carl-Daniel Hailfinger]

Am 18.06.2011 23:12 schrieb Uwe Hermann:
> On 15 October 2010 01:47, Sean Nelson  wrote:
> > Acked-by: Sean Nelson
>
> Acked-by: Uwe Hermann

Thanks, committed in r1352.

Regards,
Carl-Daniel