Re: [Mingw-users] C strings...

[Keith M. <kei...@to...>]

Hendrik Sattler wrote:
>>    char *getSubString( char *str, char *start, char *end )
> 
> Maybe write
> char *getSubString( const char *str, const char *start, const char *end 
)
> ? Do you really need to modify the arguments? No.

Agreed. The 'const' keywords make sense here, although the example code
will work just as well without them.  Of course, they do give the compiler
better hints for optimisation, in the calling code.

>>    {
>>       char  save;     /* temporary -- note NOT a pointer */
>>       char *retstr;   /* we'll use this to return the result */
> 
> ^Those two are not needed.

They *are* if you are concerned about standardisation and portability.
You avoid this requirement by using strndup() later, but that isn't
portable ('man strndup' on my GNU/Linux box says it's a GNU extension).
I suggested strdup(), which is ubiquitous, and my use *does* require
these temporaries.

>>       char *ptr1 = strstr( str, start );  /* find start mark */
>>       char *ptr2 = strstr( str, end );    /* and end mark */
> 
> You probably want end after start, so this should read:
> char* ptr1 = strstr(str,start);
> char* ptr2 = NULL;
> if (ptr1 == NULL) return strdup("");
> ptr1 += strlen(start);
> //wenn man start fest kodiert, sollte man stattdessen sizeof-1 nehmen
> ptr2 = strstr(ptr1,end);
> if (ptr2 == NULL) ptr2 = ptr1+strlen(ptr1);

You *could* write it this way -- no reason to say you *should* ;-)
Agreed, this is a worthwhile optimisation; obvious when you see it
written -- thanks for pointing it out, although I think I would
prefer to write

   ...
   if( ptr1 == NULL )
     return NULL;
   ...

rather than fragment the heap with the empty string created by

   ... strdup("");

>>       /* TODO: should check here, to ensure 'ptr2' > 'ptr1'
>>        *       and fail if not */
> 
> Not necessary.

Agreed, once you write the above optimisation.

>>       save = *ptr2;             /* save first char in trailer */
>>       *ptr2 = '\0';             /* chop trailer in situ */
>>       retstr = strdup( ptr1 );  /* make copy for return */
>>       *ptr2 = save;             /* reinstate original string */
> 
> That's not necessary:
>    return strndup(ptr1,ptr2-ptr1);

And if you are using a non-GNU compiler, then you may have to provide your
own implementation of strndup() -- using strdup(), as I suggested, is more
portable; for example, your suggestion will not work with the Sun C 
compiler
suite, which I am obliged to use on occasions.

> This way, the original string is not modified at all.
> NULL is now only returned if something goes wrong with strdup/strndup 
and 
> errno can be used. An allocated string is returned in any case but it 
might 
> have a length of zero.
> 
>>    }

--

Regards,
Keith.