[Frugalware-devel] pacman_parse_config patch

Miklos Vajna vmiklos at frugalware.org
Sun Dec 27 18:31:43 CET 2009


On Sun, Dec 27, 2009 at 03:31:29AM -0600, James Buren <ryuo at frugalware.org> wrote:
> I've created a patch that I feel is a tad faster than the old code it
> replaces.

That's not about feelings, do benchmarks if you want to prove that new
code is faster.

> If you agree, please apply this patch. It replaces a call to
> strlen(line) with line[0] == '\0' to check for an empty line. This makes
> sense because if the line truely is empty, it will be 0 length, and if so,
> the first entry will have to be the null terminator ('\0'), so why not
> just check for this and avoid the function call overhead?

You obviously did not compare the asm output before and after your
patch:

~/git/pacman-g2/lib/libpacman$ diff -u pacman.s.orig pacman.s
--- pacman.s.orig       2009-12-27 18:25:36.000000000 +0100
+++ pacman.s    2009-12-27 18:26:01.000000000 +0100
@@ -11665,8 +11665,7 @@
        movl    %eax, (%esp)
        call    _pacman_strtrim
        .loc 1 1204 0
-       leal    -4153(%ebp), %eax
-       movzbl  (%eax), %eax
+       movzbl  -4153(%ebp), %eax
        testb   %al, %al
        je      .L411
        movzbl  -4153(%ebp), %eax

As you can see the compiler already optimized away the function call, so
your patch did not really avoided any function call. ;)

Anyway it made the code a bit more readable - the old version might
suggest that the author thought strlen() accepts NULL as an argument,
which is not the case - so I applied it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: </pipermail/attachments/20091227/f2dcabd8/attachment.asc>


More information about the Frugalware-devel mailing list