strupr Postmortem Analysis

[lovecreatesbeauty]

char *strupr(char *string)
{
// char *s;
if (string)
{
// for (s = string; *s; ++s)
for (string; *string; ++string)
// *s = toupper(*s);
*string = toupper(*string);
}
return string;
}

The revised version also does convert the string to upper case. The
converted string can be accessed through the parameter pointer while
the returned pointer is located at the end of that string.

lovecreatesbeauty

 

[Richard Heathfield]

(e-mail address removed) said:

1 #include <assert.h>

You don't use this, so you could just remove it.

By the way, the line numbers are a nuisance, since they prevent us from
easily compiling your code to syntax-check it.

2 #include <stdio.h>

This line brings in various prototypes and stuff for standard I/O. It's fine
- no problem with it.

What you are missing, though, is a prototype for toupper. I recommend that
you do this as well:

#include <ctype.h>

This will give you the necessary prototype for toupper.

3
4 char *StringUp(char *p)

This line declares that StringUp is a function that takes a pointer to a
character and returns a pointer to a character. It's fine - no problem
here.

5 {

This line begins the definition of the function body.

6 char *string_start=p;

This line defines a pointer to char, and initialises it with the value
passed via the parameter p.

7 while(*p=toupper(*p))++p;

This line needs a slight modification. Your function has no guarantee that
the characters it is processing are representable as unsigned char, so it
would be wiser to write the line as follows:

while(*p=toupper((unsigned char)*p))++p;

It would be wiser still to separate the loop body line from the controlling
line, as this makes the code a little easier to read:

while(*p=toupper((unsigned char)*p))
++p;

And it would be fantastically wise to use a compound statement (although it
is not, strictly speaking, required):

while(*p=toupper((unsigned char)*p))
{
++p;
}

because, in the general case, it eases the process of modifying the loop to
incorporate any extra statements that you might require at some later
point.

8 return string_start;

This is fine - you're returning the value you passed originally, which you
carefully copied into string_start at the beginning of your function.

9 }

This line closes the function definition.

10
11 int main(void)

This line declares main to be a function taking no parameters and returning
an int. No problem here.

12 {
13 char array[]="The man walked by the 3rd door and turned
right.";

C doesn't support the wrapping of string literals around a newline
character. I realise that your code probably doesn't have this problem -
it's just something that happened in your news client. Still, it makes
compiling the code at this end a bit of a nuisance. If you have very long
string literals, you can fold them like this:

char array[]="The man walked by the 3rd"
" door and turned right.";

Note that there is no ,,,comma,,, in between the two lines.

The C preprocessor takes this absence of non-whitespace separators as a sign
that we're talking about just one string literal, so it glues the pieces
together for you - it's just as if you'd written:

char array[]="The man walked by the 3rd door and turned right.";

14 printf("--> %s\n",StringUp(array));
15 }

Ok, let's now say the new code is in lines 1-15 above (thanks Roland!).

If in line 8 I have "return string_start;", I get my desired results.
Is what is being returned a pointer?
Yes.

I thought pointers had to be prefaced with an "*". Yes?

When you declare them, yes. In use, though, it depends on what you want.

In your case, you want the pointer value itself, not the thing pointed to.

return string_start;

means "return the value this pointer has", which is precisely what you need.

return *string_start; /* no! */

would mean "return the value of the character pointed to by string_start",
which is not what you want.

Is there some way I can printf the memory addresses of the content of
*string_start?

*string_start is a single character.

string_start is a single pointer value (colloquially, an address).

Which do you mean?

 

[Jack Klein]

Robert Gamble posted:




This would imply that the integer at address "p" might be negative.

If the integer is negative, then do we not have a error? Forcing the
negative number to be positive won't solve the problem.

If anything, would the following not be better:

#include <assert.h>
#include <ctype.h>

void StringUp( char *p )
{
do assert( *p >= 0 );
while( *p = toupper( *p ), *p++ );

^^^^

Are you aware that you don't need to dereference a pointer to
increment it?

}

#include <stdio.h>

int main(void)
{
char array[] = "The man walked by the 3rd door and turned right.";

StringUp(array);

puts(array);
}

 

[Frederick Gotham]

Jack Klein posted:

^^^^

Are you aware that you don't need to dereference a pointer to
increment it?

Yes.

The pointer is dereferenced so that what it points to can be used as the
conditional expression for the while loop.