jdl@purdue.UUCP (Jon Loeliger) (05/16/86)
OK, I'm sorry, I've got to flame the string2 set of code that
John Antypas submitted to the net a few days ago. First, let
me give credit whehere credit is due. John is right, the strings
library probably needs some additional routines. He got the
shar right. He posted it correctly. Unfortunately, I'd have to
say it was pretty poor code considering it was offered as a general
improvement and tool. Let me point out some bugs and make some comments.
I compiled the code and tested it. It was very clear that the author
had done only minimal nice-case testing. For example, if strindex() is
called with target string "foo bar baz" and we wish to locate string
"fim" within it, the code not only fails, it dumps core. Here's why:
0 n = strlen(t);
1 for (loop=0;s+loop != '\0'; loop++)
2 if (strncmp(s+loop, t, n) == 0) { return(loop); }
3 return -1;
First, -10 for not indenting the subordinate if in line 2. Also, -5
for keeping the return on line 2. To be consistent with index(),
I think this routine should return a char *, -10; If the test cases
are nice (ie, searched string is present in target string), then the
for loop is a lot of noise, and the strncmp() will always succeed at
some position in s. On the other hand, if the strncmp() will always
fail, the loop will crawl off the end of the string s because the student
forgot to dereference (s+loop) in the for loop termination, line 1, -65.
Style, -10. Total : 0
Classic bugs in strreplace(): Notice how we forgot to count the '\0'
byte in the malloc? This routine will fail if strindex() returns -1.
(Good thing we checked for it :-)
.... if ( (f = strindex(s,p)) == -1) { ....
Also, Are we sure we want to copy the newly generated string back
into s's space? I doubt it. That would only be possible if the
string had become smaller... Perhaps we wanted to return m, the
malloc()'ed space.
> char *strreplace(s,p,r)
> {
>
> m = malloc( strlen(s) + strlen(r) - strlen(p) );
>
>>> f = strindex(s,p);
> strncpy(m,s,f); /* Copy up to p. */
> strcat(m,r); /* Add in r. */
>
> /* Now add the part after the replacement */
> strcat(m,s+f+strlen(p));
> /* Copy it back to s */
>>> strcpy(s,m);
>>> free(m);
> return(s);
> }
I never got to the last of the three routines...
> I don't know why these weren't included in strings(3).
>
> John Antypas -- ...!sdcsvax!jantypas
I know why these weren't included in strings(3).
Not afraid to improve the environment,
Jon Loeliger
jdl@purdue