[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [PATCH] Cleanup use of strlen() to check for empty string

  • To: "Jacob Gorm Hansen" <jacobg@xxxxxxx>, "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
  • From: "Petersson, Mats" <mats.petersson@xxxxxxx>
  • Date: Wed, 5 Oct 2005 12:51:13 +0200
  • Delivery-date: Wed, 05 Oct 2005 10:57:04 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcXJmQIEfuMP9oFIQ42kNyJ5byYSmAAATKsw
  • Thread-topic: [Xen-devel] [PATCH] Cleanup use of strlen() to check for empty string

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx 
> [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of 
> Jacob Gorm Hansen
> Sent: 05 October 2005 11:45
> To: Keir Fraser; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] Cleanup use of strlen() to 
> check for empty string
> On 10/5/05, Keir Fraser <Keir.Fraser@xxxxxxxxxxxx> wrote:
> >
> > On 4 Oct 2005, at 23:50, Jacob Gorm Hansen wrote:
> >
> > > I came across checks for strlen(s)==0 a few places in drivers/xen.
> > > Here is a patch to fix that.
> >
> > Isn't your patch exactly equivalent? None of that code is time 
> > critical enough that a call to strlen matters.
> It's just gross (looping over the entire length of a string 
> just to see if the first char is empty) that's all. Apply if you will.

I can see the argument both ways. A statement containing (strlen(s) ==
0) is very obvious what the meaning is. The compiler may even optimise
it into the form you've done (if it's clever enough).

On the other hand, if the compiler doesn't understand that (strlen(s) ==
0) is the same as (*s == 0), the code you've supplied would generate
shorter code (and run faster), which considering the number of calls to
strlen in the patch would give a decent benefit in code-size, and since
the code isn't speed critical (according to Keir), size is of more
benefit than speed.  

I created a function:

int foo(char *s)
        if (strlen(s)) return 1; else return 0;

Compiled with gcc 4.0.0 as:
gcc -O3 -s x.c (or gcc -O2 ...)
Gives the following code:

        xorl  %eax, %eax
        cmpb  $(0), (%rdi)
        sete  %al

So, assuming the compiler is still equally clever on more complex
varations of this code, it should end up with exactly the same code as
your patch. 

So although you've tried to optimise the code, it actually ends up being
nothing gained, since the compiler already can figure out what you
ACTUALLY wanted... 

Of course, other versions of gcc may not be as clever... :-(

> Jacob
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.