Comments on my code?

[Peter J. Holzer]

main(argc, argv)
int argc; char **argv;
{
char *strtrim(), *r=0;
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
free(r);
}

/* trims initial whitespace */
char *strtrim(s)
char *s;
{
char *r, *malloc(), *strcpy();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
if(r)
strcpy(r,s);
return r;
}
[...]
On the contrary, I've declared malloc correctly

No, you haven't.

and store its return value in a char *.

I'm not sure what to do about people's advice that some of the syntax in
the lecture notes is now old-fashioned - as it still seems to compile
just fine, I'm tempted to stick with what I know.

It *seems* to compile fine. It also *seems* to execute fine on your
system. But it's still wrong, and works only by chance. It may not work
on a different system.

For example, you haven't defined strlen, so the compiler has to assume
it returns an int. But it returns a size_t and a size_t may be returned
differently than an int (for example, on the 8086, in some memory
models, a size_t was returned in a pair of registers, but an int in only
one register. Declaring strlen as an int instead of a size_t would yield
wrong results if the string was longer than 32767 characters).

Then you pass the int which is the result of strlen(s)+1 to malloc.
Since no prototype is in scope, the compiler has to assume that malloc
expects an int, when really it expects a size_t. Using the example of
the 8086 compiler again, malloc would take 4 bytes off the stack, but
strtrim put only 2 bytes there, so the other two bytes contain some
random garbage (maybe the uninitialized value of r, or a part of the
value of s from the call to isspace). So malloc may try to allocate some
ridiculous amount of memory (basically rand() * 65536 + strlen(s)+1) and
fail. So strtrim also fails.

(This is harder to demonstrate with 64 bit processors: The calling
convention for x86_64 is to pass the first 6 parameters in registers, so
an int/size_t mismatch matters only if it happens in 7th or later
parameter. But assuming you can be sloppy if you only have functions
with 6 or less parameters sounds like an extremely bad idea to me)

As an erstwhile regular in this group wrote: "If you lie to the compiler
it will get its revenge!"

hp

 

[Harald van =?UTF-8?B?RMSzaw==?=]

Am I right in thinking that first standard promotions would apply, and
only then would the result be converted to size_t?

Compare these functions:

unsigned short f1 (x)
unsigned short x;
{
return x;
}

unsigned short f2 (unsigned short x)
{
return x;
}

int main(void) {
unsigned short x = 3;
return f1(x) - f2(x);
}

The call to f1 results in x being promoted to int (or unsigned int,
depending on the system), and then converted back to unsigned short. The
call to f2 does not cause x to undergo any promotion to int. The given
declaration of malloc falls in the same category as f2, not f1. However, I
would be very surprised to find an implementation for which the end result
is any different for integer types.

 

[Francine.Neary]

Compare these functions:

unsigned short f1 (x)
unsigned short x;
{
return x;

}

unsigned short f2 (unsigned short x)
{
return x;

}

int main(void) {
unsigned short x = 3;
return f1(x) - f2(x);

}

The call to f1 results in x being promoted to int (or unsigned int,
depending on the system), and then converted back to unsigned short. The
call to f2 does not cause x to undergo any promotion to int.

That's interesting... so there could in fact be situations when using
K&R style functions could shorten your code, if you were able to take
advantage of edge effects for promotion versus conversion.

 

[Barry Schwarz]

On the contrary, I posted my code, but here it is again:

main(argc, argv)
int argc; char **argv;
{
char *strtrim(), *r=0;
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
free(r);
}

/* trims initial whitespace */
char *strtrim(s)
char *s;
{
char *r, *malloc(), *strcpy();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
if(r)
strcpy(r,s);
return r;
}

(I've made one fix, by putting in the correct return type for strcpy)



On the contrary, I've declared malloc correctly and store its return
value in a char *.

No you didn't. If your notes are as old as they appear, they may say
malloc returns a char*. Unfortunately, that hasn't been true for over
15 years.

I'm not sure what to do about people's advice that some of the syntax in
the lecture notes is now old-fashioned - as it still seems to compile
just fine, I'm tempted to stick with what I know.

Old need not be bad but in this case it is hurting you more than
helping.


Remove del for email

 

[jaysome]

What happens when you call strlen(NULL)?

The C Standard says this about the function strlen:

<quote>
1 #include <string.h>
size_t strlen(const char *s);

2 The strlen function computes the length of the string pointed to by
s.

3 The strlen function returns the number of characters that precede
the terminating null character.
</quote>

Since a NULL pointer points to "nothing", it cannot point to anything
that includes a "terminating null character". Therefore, the behavior
(of strlen(NULL)) is undefined.

Best regards

 

[Barry Schwarz]

Hi-

I am learning C from some old lecture notes, but they don't have
solutions so I'd like some feedback on my code if anyone has the time.

This is an exercise: "Write a program to trim any leading whitespace
from a string and return a newly allocated buffer containing the trimmed
string. Don't forget to handle errors."

main(argc, argv)
int argc; char **argv;

The lecture notes must be very old. This method of definition, while
still legal, fell out of style before 1990.

{
char *strtrim(), *r=0;
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");

Since the requirement did not specify handling errors in a user
friendly way, I guess this qualifies.

free(r);
}

/* trims initial whitespace */
char *strtrim(s)
char *s;
{
char *r, *malloc();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);

Since there is no prototype for strlen, a C89 compiler is required to
assume that it returns an int. Since the previous declaration for
malloc was not a prototype, the compiler is required to assume it
takes an int for its only argument. Since malloc actually takes a
size_t which need not be an (unsigned) int, this invokes undefined
behavior.

It is only the coincidence that char* and void* must have the same
representation that might prevent further undefined behavior. If your
compiler and run-time library treat void* special, such as returning
it in a unique register, that prevention just evaporated.

Why are you lying to the compiler and not including the headers with
the correct prototypes?

if(r)
strcpy(r,s);
return r;
}

This looks OK to me and works correctly, but the compiler produces some
mysterious warnings about conflicting definitions that I don't really
understand.

How are we supposed to explain them if you won't tell us what they
say?


Remove del for email

 

[Flash Gordon]

Barry Schwarz wrote, On 02/09/07 17:27:

The lecture notes must be very old. This method of definition, while
still legal, fell out of style before 1990.

Well, it took a little while for all implementations to support the ANSI
standard, but basically true.

Since the requirement did not specify handling errors in a user
friendly way, I guess this qualifies.


Since there is no prototype for strlen, a C89 compiler is required to
assume that it returns an int.

True, and it means the call invokes undefined behaviour.

Since the previous declaration for
malloc was not a prototype, the compiler is required to assume it
takes an int for its only argument.

Misleading, although true in this case. The compiler is required to
assume that it takes whatever it is given. As the compiler has assumed
strlen returns an int it wll make the value of strlen(s)+1 an int as
well and so assume malloc takes an int. Had there been a correct
prototype for strlen in place then it would have been "legal" although
very bad practice. Well, legal if the return type of malloc had been
specified as void* instead of incorrectly being specified as char*.

Since malloc actually takes a
size_t which need not be an (unsigned) int, this invokes undefined
behavior.
True.

It is only the coincidence that char* and void* must have the same
representation that might prevent further undefined behavior. If your
compiler and run-time library treat void* special, such as returning
it in a unique register, that prevention just evaporated.

Why are you lying to the compiler and not including the headers with
the correct prototypes?

Because he is working from notes that pre-date the standard?

How are we supposed to explain them if you won't tell us what they
say?

Well, of you have probably explained a lot of the problems the compiler
was warning about

 

[Barry Schwarz]

Am I right in thinking that first standard promotions would apply, and
only then would the result be converted to size_t?

Since 42 is already an integer, what standard promotions are you
referring to? If the argument had been an expression with operators,
the final value of the expression would have been converted to size_t.
If the process of evaluating the expression required standard
promotions, they would be performed first. However, the expression
consisted of a single short int variable, then I don't believe the
standard promotions are needed to convert the short value to a size_t
value (n1124, section 6.3.1.1, footnote 48).


Remove del for email

 

[Keith Thompson]

[...]

Am I right in thinking that first standard promotions would apply, and
only then would the result be converted to size_t?

Since 42 is already an integer, what standard promotions are you
referring to?

[...]

Ahem. Yes, 42 is an integer, but the relevant point is that it's an
int. (int is one of several integer types.)

 

[pete]

[...]

Am I right in thinking that first
standard promotions would apply, and
only then would the result be converted to size_t?

Since 42 is already an integer, what standard promotions are you
referring to?

[...]

Ahem. Yes, 42 is an integer, but the relevant point is that it's an
int. (int is one of several integer types.)

But, there are no promotions
that would apply to an argument expression of type int,
prior to conversion to the parameter's type of size_t,
as in what Francine.Neary replied to:

"If you had declared it correctly as
void *malloc(size_t);
then a call like malloc(42) would implicitly convert the argument to
size_t -- and a call with a non-numeric argument would trigger an
error message."

 

[Keith Thompson]

[...]
Am I right in thinking that first
standard promotions would apply, and
only then would the result be converted to size_t?

Since 42 is already an integer, what standard promotions are you
referring to?

[...]

Ahem. Yes, 42 is an integer, but the relevant point is that it's an
int. (int is one of several integer types.)

But, there are no promotions
that would apply to an argument expression of type int,
prior to conversion to the parameter's type of size_t,
as in what Francine.Neary replied to:

[snip]

My point was that Barry was glossing over the disinction between 'int'
and 'integer'.