From: rbs{at}snippets.org
To: c_echo{at}yahoogroups.com
To: tomtorfs{at}village.uunet.be
Copy: c_echo{at}yahoogroups.com

Quoting tomtorfs{at}village.uunet.be:

> This file contains a function that demonstrates a common programming
> mistake (hidden in the macro LAST_CHAR):
>
> static char *StripTrailingSpaces(char *string)
> {
>       if (!string || (0 == strlen(string)))
>             return NULL;
>
>       while (isspace(LAST_CHAR(string)))
>             LAST_CHAR(string) = NUL;
>
>       return string;
> }
>
> from sniptype.h:
>
> #define LAST_CHAR(s) (((char *)s)[strlen(s) - 1])
>
> So this function (StripTrailingSpaces) calls strlen() twice for each
> trailing space character in the string, and for those that don't know,
> strlen() has  to walk through the string character by character from the
> beginning looking for the trailing nul-terminator. This is just a silly waste
> of CPU cycles and time.

I'm not exactly sure who wrote that, but it was probably me, so I'll answer...

You're right, of course, it is wasteful. In my defense, I can only point out a
couple of justifications:

1.  Many times, string manipulation is used in a context of the user interface
    where the speed at which data arrives is vastly slower than the CPU, so
    efficiency isn't an issue.

2.  Even when efficiency is an issue, we rarely write to wring the last drop of
    memory savings or speed out of our programs, since hardware is so cheap
    compared to or time. If performance is *that* much of an issue, we should
    probably be working in assembly rather than a HLL.

3.  In any discussion of efficient coding, the first issue is always, "How fast
    is fast enough?" If we write something - even something wasteful or hastily
    thrown together - and it runs fast enough, then it's defensible to consider
    ourselves done.

4.  The LAST_CHAR() is a truly handy macro, usable as either an lvalue or an
    rvalue, which this function demonstrates.

The solution, obviously, would have been to use rmtrail(), also from SNIPPETS. I
can't say now why I didn't use it at the time, but since what I was using it for
ran fast enough, it was OK at the time.

> Also, the initial check for a 0-length string performs yet-another strlen()
> on the string in question. The strlen()-scan of the string is not necessary to
> determine if it is 0-length or not. Simply checking the first element of the
> array (or the first character pointed to) will suffice.
>
> Here is a much faster and more efficient version of the function:
>
> static char *StripTrailingSpaces(char *string)
> {
>         size_t len;
>
>         if(string == NULL || *string == 0)
>                 return string; /* why would you return NULL if empty? */
>
>         len = strlen(str);
>         while(len && isspace(string[len-1]))
>                 len--;
>
>         if(string[len] != 0)
>                 string[len] = 0; /* Don't re-terminate, could be static str
> */
>
>         return string;
> }

OK, let's compare this to rmtrail.c from SNIPPETS (the header and C++ bits
elided for clarity)...

----[ snip ]----
#include 
#include 

char *rmtrail(char *str)
{
      int i;

      if (str && 0 != (i = strlen(str)))
      {
            while (--i >= 0)
            {
                  if (!isspace(str[i]))
                        break;
            }
            str[++i] = NUL;
      }
      return str;
}
----[ snip ]----

As you can see, they're essentially identical, with a few differences:

1.  First of all, we both test for a null pointer. However, you next test the
    initial character, and only then call strlen(). Your way is slightly faster,
    but I see little algorithmic reason not to combine the two operations since
    the strlen() call will also effectively test for a leading NUL character.

2.  The speed you gained by the explicit test for NUL before the while() loop,
    you now give back with a questionable test for NUL after the while() loop. I
    also don't understand your comment about static pointers. The call to
    strlen() guaranteed that all the chars you passed over in the while() loop
    were non-NUL, so this test seems totally extraneous.

3.  An obvious flaw in both functions is the chance that the buffer will be
    under-run. After all, we have no assurance that the byte in memory just
    prior to the beginning of the string isn't NUL.

I'll fix #3 for rmtrail.c this weekend... ;-)

-------------------------------------------------------------
Consulting: http://www.MicroFirm.biz/
Web graphics development: http://Image-Magicians.com/
Software archives: http://snippets.snippets.org/
  c.snippets.org/   cpp.snippets.org/      java.snippets.org/
  d.snippets.org/   python.snippets.org/   perl.snippets.org/
  dos.snippets.org/ embedded.snippets.org/ apps.snippets.org/
Audio and loudspeaker design:
  http://LDSG.snippets.org/   http://www.diyspeakers.net/


-------------------------------------------------
This mail sent through IMP: http://horde.org/imp/

--- SoupGate-Win32 v1.05