You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@bellsouth.net> on 2001/05/23 16:20:37 UTC

Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

trawick@apache.org writes:

> trawick     01/05/23 07:15:55
> 
>   Modified:    .        CHANGES libapr.dsp apr.dsp
>                include  apr_strings.h
>                strings  Makefile.in
>                test     .cvsignore Makefile.in
>   Added:       strings  apr_strtok.c
>                test     teststr.c
>   Log:
>   Add apr_strtok(), a thread-safe flavor of strtok() which has the
>   same interface as strtok_r().

I mentioned before that I was going to suck in the FreeBSD strtok_r()
implementation.  Unfortunately, when I looked at the license it didn't
quite fit the familiar pattern.  Somebody I'd never heard of
("Softweyr LLC") actually owns the copyright and an advertising clause
remains in the license, so I reinvented the wheel.  My C coding isn't
quite as bad as my legalese.

Some eager person may wish to replace the strchr() calls with inline
code.  The only compiler I'm really familiar with w.r.t. code
generation will replace strchr() with inline code but I suspect that
is not the case with most compilers.

The MSVC .dsp changes are untested (duck!).  I'm somewhat sorry about
that :)

Oh, and I guess I just broke httpd.exp :)  I'll handle that now.

-- 
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Jeff Trawick <tr...@bellsouth.net>.
Jeff Trawick <tr...@bellsouth.net> writes:

> >   Add apr_strtok(), a thread-safe flavor of strtok() which has the
> >   same interface as strtok_r().
...
> Some eager person may wish to replace the strchr() calls with inline
> code.  The only compiler I'm really familiar with w.r.t. code
> generation will replace strchr() with inline code but I suspect that
> is not the case with most compilers.

IMHO, the usefulness of replacing the strchr() calls with inline code
has nothing to do with the efficiency of the strchr() in the C
library.  Whether or not the C library strchr() it is written in
assembler, I'm willing to bet that it is fast.

I think the usefulness of having inline code in this situation is
because:

1) function call overhead is gone (on at least one platform/compiler I
   know, the cost of the function call is >= than the cost of
   strchr())

2) the code for strchr() is visible when the calling function is
   optimized, leading to better code than when part of the work has to
   be done in code the optimizer won't see

(This being said, I don't personally intend to look at this code again
unless it breaks or we start calling apr_strtok() all over the place
:) )

-- 
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Marr <gr...@alum.wpi.edu>.
At 10:35 AM 05/24/2001, William A. Rowe, Jr. wrote:
>From: "Greg Marr" <gr...@alum.wpi.edu>
>Sent: Thursday, May 24, 2001 7:59 AM
>
> > That's actually what I was expecting to find.  I was quite 
> surprised
> > to find it in C.  That was from MSVC by the way.
>
>What are you talking about :-?  See 
>DevStudio\VC\crt\src\intel\STRCHR.ASM

Ah, I must not have searched in .ASM files.  They apparently have it 
in both forms.

-- 
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by "William A. Rowe, Jr." <ad...@rowe-clan.net>.
From: "Greg Marr" <gr...@alum.wpi.edu>
Sent: Thursday, May 24, 2001 7:59 AM


> At 03:47 AM 05/24/2001, Greg Stein wrote:
> >No matter how much you may optimize at the *C* level, I doubt that you'll
> >match what various C libraries will do. Anybody worth their salt would code
> >strchr() in assembly (and fall back to a C implementation if there isn't a
> >selected ASM version for the host platform).
> 
> That's actually what I was expecting to find.  I was quite surprised 
> to find it in C.  That was from MSVC by the way.

What are you talking about :-?  See DevStudio\VC\crt\src\intel\STRCHR.ASM

Bill


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Marr <gr...@alum.wpi.edu>.
At 03:47 AM 05/24/2001, Greg Stein wrote:
>No matter how much you may optimize at the *C* level, I doubt that 
>you'll
>match what various C libraries will do. Anybody worth their salt 
>would code
>strchr() in assembly (and fall back to a C implementation if there 
>isn't a
>selected ASM version for the host platform).

That's actually what I was expecting to find.  I was quite surprised 
to find it in C.  That was from MSVC by the way.

>btw, function open/close braces are *always* at column zero in 
>Apache code.
>Most other style stuff can be flexed, but not that one. :-)

That's not Apache code.  There are a lot of differences between my 
style and Apache style.  :)

-- 
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Stein <gs...@lyra.org>.
On Thu, May 24, 2001 at 12:47:18AM -0700, Greg Stein wrote:
>...
> btw, function open/close braces are *always* at column zero in Apache code.
> Most other style stuff can be flexed, but not that one. :-)

Heh. /me thinks I've been coding in C too long. "column one" might be a bit
more *human*

:-)

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Stein <gs...@lyra.org>.
No matter how much you may optimize at the *C* level, I doubt that you'll
match what various C libraries will do. Anybody worth their salt would code
strchr() in assembly (and fall back to a C implementation if there isn't a
selected ASM version for the host platform).

Then, you have the whole "compiler can detect strchr()" aspect. I believe
that GCC recognizes it. ... Yup. I see an ASM version on my system.

btw, function open/close braces are *always* at column zero in Apache code.
Most other style stuff can be flexed, but not that one. :-)

Cheers,
-g

On Wed, May 23, 2001 at 07:55:06PM -0400, Greg Marr wrote:
> At 06:33 PM 05/23/2001, Greg Stein wrote:
> >And we *should* use strchr(). Unwrapping that into a loop is bogus. 
> >Even if
> >the compiler doesn't optimize it, the C library has highly optimized
> >versions which isn't going to happen with our own implementation.
> 
> Since I was curious, I went looking for one:
> 
> char * strchr (
>          const char * string,
>          int ch
>          )
> {
>          while (*string && *string != (char)ch)
>                  string++;
> 
>          if (*string == (char)ch)
>                  return((char *)string);
>          return(NULL);
> }
> 
> Well, it's somewhat optimized, but not quite how I would have written 
> it, and it's horribly formatted.
> 
> char * strchr(const char *string, int ch)
>      {
>      while(*string && (*string != (char)ch))
>          string++;
> 
>      return(*string ? (char *)string : NULL);
>      }
> 
> Ah, much better.
> 
> -- 
> Greg Marr
> gregm@alum.wpi.edu
> "We thought you were dead."
> "I was, but I'm better now." - Sheridan, "The Summoning"

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Marr <gr...@alum.wpi.edu>.
At 06:33 PM 05/23/2001, Greg Stein wrote:
>And we *should* use strchr(). Unwrapping that into a loop is bogus. 
>Even if
>the compiler doesn't optimize it, the C library has highly optimized
>versions which isn't going to happen with our own implementation.

Since I was curious, I went looking for one:

char * strchr (
         const char * string,
         int ch
         )
{
         while (*string && *string != (char)ch)
                 string++;

         if (*string == (char)ch)
                 return((char *)string);
         return(NULL);
}

Well, it's somewhat optimized, but not quite how I would have written 
it, and it's horribly formatted.

char * strchr(const char *string, int ch)
     {
     while(*string && (*string != (char)ch))
         string++;

     return(*string ? (char *)string : NULL);
     }

Ah, much better.

-- 
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by "Victor J. Orlikowski" <v....@gte.net>.
 > And we *should* use strchr(). Unwrapping that into a loop is bogus. Even if
 > the compiler doesn't optimize it, the C library has highly optimized
 > versions which isn't going to happen with our own implementation.

Fine with me; all attempts at replacement withdrawn.

Victor
-- 
Victor J. Orlikowski
======================
v.j.orlikowski@gte.net
orlikowski@apache.org
vjo@us.ibm.com


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by Greg Stein <gs...@lyra.org>.
On Wed, May 23, 2001 at 03:33:29PM -0400, Victor J. Orlikowski wrote:
>  >  > Some eager person may wish to replace the strchr() calls with inline
>  >  > code.  The only compiler I'm really familiar with w.r.t. code
>  >  > generation will replace strchr() with inline code but I suspect that
>  >  > is not the case with most compilers.
>  > 
>  > Macros decided not to play nice.
>  > 
>  > If this works for you, lemme know, and I'll commit it.
> 
> Please ignore; this patch had several errors.

And we *should* use strchr(). Unwrapping that into a loop is bogus. Even if
the compiler doesn't optimize it, the C library has highly optimized
versions which isn't going to happen with our own implementation.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by "Victor J. Orlikowski" <v....@gte.net>.
 >  > Some eager person may wish to replace the strchr() calls with inline
 >  > code.  The only compiler I'm really familiar with w.r.t. code
 >  > generation will replace strchr() with inline code but I suspect that
 >  > is not the case with most compilers.
 > 
 > Macros decided not to play nice.
 > 
 > If this works for you, lemme know, and I'll commit it.

Please ignore; this patch had several errors.

Thanks,
Victor
-- 
Victor J. Orlikowski
======================
v.j.orlikowski@gte.net
orlikowski@apache.org
vjo@us.ibm.com


Re: cvs commit: apr/test teststr.c .cvsignore Makefile.in

Posted by "Victor J. Orlikowski" <v....@gte.net>.
 > Some eager person may wish to replace the strchr() calls with inline
 > code.  The only compiler I'm really familiar with w.r.t. code
 > generation will replace strchr() with inline code but I suspect that
 > is not the case with most compilers.

Macros decided not to play nice.

If this works for you, lemme know, and I'll commit it.

Index: apr_strtok.c
===================================================================
RCS file: /home/cvs/apr/strings/apr_strtok.c,v
retrieving revision 1.1
diff -u -d -r1.1 apr_strtok.c
--- apr_strtok.c	2001/05/23 14:15:44	1.1
+++ apr_strtok.c	2001/05/23 17:42:32
@@ -59,19 +59,26 @@
 #include "apr.h"
 #include "apr_strings.h"
 
-#define APR_WANT_STRFUNC   /* for strchr() */
-#include "apr_want.h"
-
 APR_DECLARE(char *) apr_strtok(char *str, const char *sep, char **last)
 {
     char *token;
+    char *tmpcmp;
 
     if (!str)           /* subsequent call */
         str = *last;    /* start where we left off */
 
     /* skip characters in sep (will terminate at '\0') */
-    while (*str && strchr(sep, *str))
-        ++str;
+    while (*str) {
+        tmpcmp = sep;
+        do {
+            if (*tmpcmp == *str)
+                break;
+        } while (*tmpcmp++) 
+        if (*tmpcmp)
+            ++str;
+        else
+            break;
+    }
 
     if (!*str)          /* no more tokens */
         return NULL;
@@ -82,8 +89,17 @@
      * prepare for the next call (will terminate at '\0) 
      */
     *last = token + 1;
-    while (**last && !strchr(sep, **last))
-        ++*last;
+    while (**last) {
+        tmpcmp = sep;
+        do {
+            if (*tmpcmp == **last)
+                break;
+        } while (*tmpcmp++)
+        if (*tmpcmp)
+            break;
+        else
+            ++*last;
+    }
 
     if (**last) {
         **last = '\0';

Victor
-- 
Victor J. Orlikowski
======================
v.j.orlikowski@gte.net
orlikowski@apache.org
vjo@us.ibm.com