468,290 Members | 1,998 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 468,290 developers. It's quick & easy.

Comments on my code?

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;
{
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();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
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 come we never talk anymore?
Sep 1 '07 #1
28 1397
Platonic Solid wrote:
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;
{
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();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
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 come we never talk anymore?
You should not declare malloc like that.
#include <stdlib.h>
and never define your own definitions for library functions.

You need
#include <string.h>
for strlen's prototype.

And:
char *strtrim(s)
char *s;
This is old fashioned C. Better is:
char *strtrim(char *s)
P.S. What happens when s is NULL?

jacob
Sep 1 '07 #2
Platonic Solid wrote:
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;
{
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();
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
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.
The program you wrote would be a correct way to write code a long time ago,
so if your lecture notes were really old, then well done. It's not a right
way to do things now, though. Try this version, and see if you understand
the changes I made:

#include <stdio.h /* include the headers that declare the library */
#include <stdlib.h/* functions and objects you are using. you should */
#include <string.h/* not declare them in your own program, especially*/
#include <ctype.h /* the wrong way; that is what caused your warning */

/*
* a forward declaration for your function should not be local to
* another function. also, you can specify the types of the parameters
* in the declaration as well as the definition. doing so allows the
* compiler to check and automatically convert the values you will be
* passing to the function.
*
* I renamed the function because technically, strtrim isn't a valid name
* for it, but the reasons why probably won't be of interest to you at
* this stage.
*/
char *trimstr(char *);

/*
* the types of the parameters should be specified in the parameter list,
* not between the ) and {. it's still valid, but has the same problems
* as not specifying the parameter types in the function declaration.
* and you should specify the return type, even if it's int.
*/
int main(int argc, char **argv) {
/*
* for readability reasons, I would recommend against your use of ?:,
* and use if statements instead.
*/
if (argc 1) {
char *r = trimstr(argv[1]);
if (!r) {
/* error messages should normally go to stderr, not stdout */
fputs("Unspecified error\n", stderr);
} else {
puts(r);
free(r);
}
}
/* main returns int (even in your original version), so return an int */
return 0;
}

char *trimstr(char *s) {
char *r;
/* for readability, you can rewrite your for loop */
while (isspace(*s))
s++;
/*
* malloc doesn't need to be declared manually here, with <stdlib.h>
* included. and malloc doesn't return 'char *' anymore, but 'void *'
* void is probably not covered in your lecture notes, but in the
* context of pointers, it means a pointer to any object, but you
* don't have the object's type.
*/
r = malloc(strlen(s)+1);
if(r)
strcpy(r,s);
return r;
}
Sep 1 '07 #3
On 1 Sep 2007 at 12:53, jacob navia wrote:
P.S. What happens when s is NULL?
If you look carefully I check argc>1, so it never gets called with s
null.

--
How come we never talk anymore?
Sep 1 '07 #4
On Sat, 01 Sep 2007 14:01:58 +0200, Platonic Solid wrote:
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;
In modern C this is usually written as
int main(int argc, char *argv[])
But the old form continues to work, even it "looks strange".
{
char *strtrim(), *r=0;
Better put function declaration at file scope, and use NULL
defined in <stddef.hand other headers instead of 0. Of course
your form will work, too, but it is "ugly".
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
To somebody which doesn't remember all the precedence rules by
heart, this could be confusing. If you *really* think that if/else
is evil (it isn't, at least it'd allow you to write the error
message to stderr), at least put parentheses around the first
operand of ?: ...
<nitpick pedantry="high">Nowadays puts() takes a const char *,
but you're passing it a char *, because there is no prototype in
scope.</nitpickEven if it is overwhelmingly likely to work,
usually one use the prototype for puts() found in <stdio.h>.
free(r);
}

/* trims initial whitespace */
char *strtrim(s)
char *s;
{
char *r, *malloc();
In modern C malloc returns a void*, not a char*. Also the argument
is a size_t, see FAQ 7.15.
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
strlen returns a size_t, but here you're declaring it as returning
an int.
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.
See FAQ 11.29a for compatibility problem between old and new C.
--
Army1987 (Replace "NOSPAM" with "email")
No-one ever won a game by resigning. -- S. Tartakower

Sep 1 '07 #5
Platonic Solid wrote:
On 1 Sep 2007 at 12:53, jacob navia wrote:
>P.S. What happens when s is NULL?

If you look carefully I check argc>1, so it never gets called with s
null.

--
How come we never talk anymore?
Sure, but is strtrim only for this particular call?
In that case a function is not necessary!
Sep 1 '07 #6
On 2007-09-01 13:26, jacob navia <ja***@jacob.remcomp.frwrote:
Platonic Solid wrote:
>On 1 Sep 2007 at 12:53, jacob navia wrote:
>>P.S. What happens when s is NULL?

If you look carefully I check argc>1, so it never gets called with s
null.

Sure, but is strtrim only for this particular call?
In that case a function is not necessary!
What happens when you call strlen(NULL)?

hp
--
_ | Peter J. Holzer | I know I'd be respectful of a pirate
|_|_) | Sysadmin WSR | with an emu on his shoulder.
| | | hj*@hjp.at |
__/ | http://www.hjp.at/ | -- Sam in "Freefall"
Sep 1 '07 #7
On 2007-09-01 12:59, Harald van Dijk <tr*****@gmail.comwrote:
Platonic Solid wrote:
>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.
[ lots of good advice snipped]

char *trimstr(char *s) {
char *r;
/* for readability, you can rewrite your for loop */
while (isspace(*s))
s++;
isspace expects an argument in the range [EOF, 0 .. UCHAR_MAX]. But
*s will be in the range [CHAR_MIN .. CHAR_MAX], so isspace may be called
with an illegal value, resulting in undefined behaviour.

You must get the value into the correct range first:

while (isspace((unsigned char)*s))
s++;

hp

--
_ | Peter J. Holzer | I know I'd be respectful of a pirate
|_|_) | Sysadmin WSR | with an emu on his shoulder.
| | | hj*@hjp.at |
__/ | http://www.hjp.at/ | -- Sam in "Freefall"
Sep 1 '07 #8
Platonic Solid wrote:
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.
Judging from your code, these are extremely old lecture notes. If there
is a date on them, I would guess it to be somewhere in the 1980's.
Since that time, C has evolved a bit. Not too much but enough that it
would be worthwhile to have a more recent book to accompany these
lecture notes.
I would suggest you get a copy of "The C Programming Language", second
edition, by Kernighan and Ritchie (often referred to as K&R2)
>
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."
To use library functions, you should #include the appropriate headers.
For this program, you need

#include <stdio.h/* for puts() */
#include <stdlib.h/* for malloc()/free() */
#include <ctype.h/* for isspace() */
#include <string.h/* for strcpy() */
main(argc, argv)
int argc; char **argv;
This style of declaring parameters is considered to be outdated.
It is preferred that you write it now like this:
int main(int argc, char **argv)
{
char *strtrim(), *r=0;
It is not wrong to declare a function at this point, but it is not
considered to be good style.
It is recommended that you declare functions at file scope, and that you
give a prototype for the function.
The prototype for strtrim() looks like
char *strtrim(char*);
The advantage of a prototype is that the compiler can verify (and
complain!) that you call the function in the correct way.

As a more personal style issue, I prefer to have one item per
declaration. I would have written the above declaration as:
char *strtrim(char*);
char *r;
I also might have used a more descriptive name for r.
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
C allows you to write extremely complex expressions, but usually it is
best to avoid using that freedom. (exception: IOCCC entries)
The statement above is right at the limit of how complex you should make
it (and some would argue it is over the limit).
free(r);
As main() returns a success/failure code to the OS, you should make sure
that it is a sensible code:
return 0; /* indicates success. Other code are EXIT_SUCCESS and
EXIT_FAILURE */
}

/* trims initial whitespace */
char *strtrim(s)
char *s;
Same comment as with main(): you should place the argument type also
within the parentheses.
char *strtrim(char *s)
{
char *r, *malloc();
You should not write a declaration for library functions, like malloc().
Instead you should #include the required header (see above).
for( ; isspace(*s); s++);
r=malloc(strlen(s)+1);
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.
Those warnings are probably due to the required headers that you forgot
to include.
Modern compilers will complain if the encounter a call to a function
that they have not yet seen before (like your calls to puts, isspace
and strcpy). The complaints are because the compiler has to make some
assumptions that are likely to be wrong.

BTW, if you have questions about compiler messages, it works so much
better if you post the compiler output (along with the code you fed
into the compiler). Most of us will know immediately what it means,
because we have seen the message before.

Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://www.eskimo.com/~scs/C-faq/top.html
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/
Sep 1 '07 #9
Peter J. Holzer wrote:
On 2007-09-01 12:59, Harald van Dijk <tr*****@gmail.comwrote:
>Platonic Solid wrote:
>>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.

[ lots of good advice snipped]

>char *trimstr(char *s) {
char *r;
/* for readability, you can rewrite your for loop */
while (isspace(*s))
s++;

isspace expects an argument in the range [EOF, 0 .. UCHAR_MAX]. But
*s will be in the range [CHAR_MIN .. CHAR_MAX], so isspace may be called
with an illegal value, resulting in undefined behaviour.

You must get the value into the correct range first:

while (isspace((unsigned char)*s))
s++;
Thanks, I forgot about that. It would be nice if I could get my system to
warn about passing a char; it's not the first time I've left out the cast.
Sep 1 '07 #10
Platonic Solid wrote:
>
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;
{
char *strtrim(), *r=0;
puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
free(r);
.... snip ...
>
This looks OK to me and works correctly, but the compiler produces
some mysterious warnings about conflicting definitions that I
don't really understand.
To start with, you are failing to use prototypes (using obsolete
K&R 1 function headers), omitting necessary #includes, and using
system reserved names. The latter includes anything of the form
"strx..." where x is a lowercase letter.

You can get a satisfactory approximation to the standard in N869 or
N1124. You can find a suitable bzip2 compressed version of N869
at:

<http://cbfalconer.home.att.net/download/>

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

--
Posted via a free Usenet account from http://www.teranews.com

Sep 1 '07 #11
Platonic Solid wrote:
>
.... snip ...
>
OK, so that explains why strcpy produces a warning as it returns
char * and not int. But the compiler also complains about malloc
(I specifically supply the return type for that) and strlen, which
returns int so should be OK by Q 1.25.
You didn't give code, so we can't make suggestions. If you have
problems with malloc you are probably a) casting the return or b)
failing to include <stdlib.hor c) capturing the return value in
other than a pointer or d) using a C++ compiler.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

--
Posted via a free Usenet account from http://www.teranews.com

Sep 1 '07 #12
On 1 Sep 2007 at 16:42, CBFalconer wrote:
Platonic Solid wrote:
>>
... snip ...
>>
OK, so that explains why strcpy produces a warning as it returns
char * and not int. But the compiler also complains about malloc
(I specifically supply the return type for that) and strlen, which
returns int so should be OK by Q 1.25.

You didn't give code, so we can't make suggestions.
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)

If you have problems with malloc you are probably a) casting the
return or b) failing to include <stdlib.hor c) capturing the return
value in other than a pointer or d) using a C++ compiler.
On the contrary, I've declared malloc correctly 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.
--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>
--
How come we never talk anymore?
Sep 1 '07 #13
Platonic Solid wrote:
char *r, *malloc(), *strcpy();

On the contrary, I've declared malloc correctly and store its return
value in a char *.
No, you haven't declared malloc correctly.
Sep 1 '07 #14
Harald van Dijk <tr*****@gmail.comwrites:
Platonic Solid wrote:
> char *r, *malloc(), *strcpy();

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

No, you haven't declared malloc correctly.
Specifically, malloc returns void*, not char*, and it takes an
argument of type size_t. Since you didn't declare the argument type,
something as simple as malloc(42) invokes undefined behavior, since
you're passing an argument of type int.

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.

But the best way to provide the correct declaration is not to
manually re-write it yourself. It's to provide
#include <stdlib.h>
at the top of your source file. There's simply no good reason not to
do it that way.

The more correct information you can provide for the compiler, the
more it can help you. A lot of the facilities for this, particularly
function prototypes, were introduced with the ANSI C standard in 1989.

Prototypes were introduced into C 18 years ago. There are no longer
any significant compilers still being used that don't support them, so
there's no good reason not to use them, along with a number of other
features introduced by ANSI. (The latest ISO C standard, C99, hasn't
caught on as well, so there are still good reasons to avoid
C99-specific features.)

Your code is archaic. You can still write working archaic code if
you're careful enough, but you really should learn the language as it
is today. Reading K&R2 is probably the best way to do this.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Sep 1 '07 #15
On Sat, 01 Sep 2007 20:35:10 +0200, Platonic Solid wrote:
On the contrary, I've declared malloc correctly 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.
But, in ANSI/ISO C, which has been around since 1989 and radically
revised in 1999 (even though this last version isn't widely
implemented yet), malloc() returns a void *, not a char * anymore.
It is very likely to work, since char* and void* are required to
have the same alignment etc., but probably a sufficiently deep
level of language lawyering would show that the behavior is
undefined by the International Standard.

Also, nowadays it is common to declare library functions with the
prototypes in the corresponding headers. For example, stdlib.h
contains a correct prototype for malloc(), and you can add it to
your code with the preprocessing directive
#include <stdlib.h>
which gets replaced with the contents of that file.
In C99 you *always* must declare functions, even if they return an
int, and doing so is a very good idea in C89, too.
--
Army1987 (Replace "NOSPAM" with "email")
No-one ever won a game by resigning. -- S. Tartakower

Sep 1 '07 #16
On Sep 1, 8:22 pm, Keith Thompson <ks...@mib.orgwrote:
Harald van D k <true...@gmail.comwrites:
Platonic Solid wrote:
char *r, *malloc(), *strcpy();
On the contrary, I've declared malloc correctly and store its return
value in a char *.
No, you haven't declared malloc correctly.

Specifically, malloc returns void*, not char*, and it takes an
argument of type size_t. Since you didn't declare the argument type,
something as simple as malloc(42) invokes undefined behavior, since
you're passing an argument of type int.

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.
Am I right in thinking that first standard promotions would apply, and
only then would the result be converted to size_t?
But the best way to provide the correct declaration is not to
manually re-write it yourself. It's to provide
#include <stdlib.h>
at the top of your source file. There's simply no good reason not to
do it that way.

The more correct information you can provide for the compiler, the
more it can help you. A lot of the facilities for this, particularly
function prototypes, were introduced with the ANSI C standard in 1989.

Prototypes were introduced into C 18 years ago. There are no longer
any significant compilers still being used that don't support them, so
there's no good reason not to use them, along with a number of other
features introduced by ANSI. (The latest ISO C standard, C99, hasn't
caught on as well, so there are still good reasons to avoid
C99-specific features.)

Your code is archaic. You can still write working archaic code if
you're careful enough, but you really should learn the language as it
is today. Reading K&R2 is probably the best way to do this.

--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Sep 1 '07 #17
Platonic Solid wrote:
On 1 Sep 2007 at 16:42, CBFalconer wrote:
>Platonic Solid wrote:
... snip ...
>>OK, so that explains why strcpy produces a warning as it returns
char * and not int. But the compiler also complains about malloc
(I specifically supply the return type for that) and strlen, which
returns int so should be OK by Q 1.25.
You didn't give code, so we can't make suggestions.

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)
Yabbut, that's the wrong way to do it, and it still
doesn't fix the incorrect declaration of strlen(). The
right fix (hint: Have you learned about the #include
directive yet?) would take care of both problems, and
would enable the compiler to catch errors like strcpy(x)
or strlen(x, y), too.
>If you have problems with malloc you are probably a) casting the
return or b) failing to include <stdlib.hor c) capturing the return
value in other than a pointer or d) using a C++ compiler.

On the contrary, I've declared malloc correctly and store its return
value in a char *.
You have declared malloc *in*correctly. The cure is
similar to that for strlen() et al.
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.
I bet your 1955 Studebaker runs just fine without those
pesky seat belts, air bags, anti-lock brakes, and emission-
control devices, too.

--
Eric Sosman
es*****@ieee-dot-org.invalid
Sep 1 '07 #18
On 2007-09-01 18:35, Platonic Solid <pl*****@mailinator.comwrote:
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

--
_ | Peter J. Holzer | I know I'd be respectful of a pirate
|_|_) | Sysadmin WSR | with an emu on his shoulder.
| | | hj*@hjp.at |
__/ | http://www.hjp.at/ | -- Sam in "Freefall"
Sep 1 '07 #19
Fr************@googlemail.com wrote:
On Sep 1, 8:22 pm, Keith Thompson <ks...@mib.orgwrote:
>Harald van D k <true...@gmail.comwrites:
Platonic Solid wrote:
char *r, *malloc(), *strcpy();
>On the contrary, I've declared malloc correctly and store its return
value in a char *.
No, you haven't declared malloc correctly.

Specifically, malloc returns void*, not char*, and it takes an
argument of type size_t. Since you didn't declare the argument type,
something as simple as malloc(42) invokes undefined behavior, since
you're passing an argument of type int.

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.

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.
Sep 1 '07 #20
On Sep 1, 10:33 pm, Harald van D k <true...@gmail.comwrote:
Francine.Ne...@googlemail.com wrote:
On Sep 1, 8:22 pm, Keith Thompson <ks...@mib.orgwrote:
Harald van D k <true...@gmail.comwrites:
Platonic Solid wrote:
char *r, *malloc(), *strcpy();
On the contrary, I've declared malloc correctly and store its return
value in a char *.
No, you haven't declared malloc correctly.
Specifically, malloc returns void*, not char*, and it takes an
argument of type size_t. Since you didn't declare the argument type,
something as simple as malloc(42) invokes undefined behavior, since
you're passing an argument of type int.
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.
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.
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.
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.
Sep 1 '07 #21
On Sat, 1 Sep 2007 20:35:10 +0200 (CEST), Platonic Solid
<pl*****@mailinator.comwrote:
>On 1 Sep 2007 at 16:42, CBFalconer wrote:
>Platonic Solid wrote:
>>>
... snip ...
>>>
OK, so that explains why strcpy produces a warning as it returns
char * and not int. But the compiler also complains about malloc
(I specifically supply the return type for that) and strlen, which
returns int so should be OK by Q 1.25.

You didn't give code, so we can't make suggestions.

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)

>If you have problems with malloc you are probably a) casting the
return or b) failing to include <stdlib.hor c) capturing the return
value in other than a pointer or d) using a C++ compiler.

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
Sep 2 '07 #22
On Sat, 1 Sep 2007 16:23:39 +0200, "Peter J. Holzer"
<hj*********@hjp.atwrote:
>On 2007-09-01 13:26, jacob navia <ja***@jacob.remcomp.frwrote:
>Platonic Solid wrote:
>>On 1 Sep 2007 at 12:53, jacob navia wrote:
P.S. What happens when s is NULL?

If you look carefully I check argc>1, so it never gets called with s
null.

Sure, but is strtrim only for this particular call?
In that case a function is not necessary!

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
--
jay
Sep 2 '07 #23
On Sat, 1 Sep 2007 14:01:58 +0200 (CEST), Platonic Solid
<pl*****@mailinator.comwrote:
>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
Sep 2 '07 #24
Barry Schwarz wrote, On 02/09/07 17:27:
On Sat, 1 Sep 2007 14:01:58 +0200 (CEST), Platonic Solid
<pl*****@mailinator.comwrote:
>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.
Well, it took a little while for all implementations to support the ANSI
standard, but basically true.
>{
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.
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?
> 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?
Well, of you have probably explained a lot of the problems the compiler
was warning about :-)
--
Flash Gordon
Sep 2 '07 #25
On Sat, 01 Sep 2007 13:06:55 -0700, Fr************@googlemail.com
wrote:
>On Sep 1, 8:22 pm, Keith Thompson <ks...@mib.orgwrote:
>Harald van D k <true...@gmail.comwrites:
Platonic Solid wrote:
char *r, *malloc(), *strcpy();
>On the contrary, I've declared malloc correctly and store its return
value in a char *.
No, you haven't declared malloc correctly.

Specifically, malloc returns void*, not char*, and it takes an
argument of type size_t. Since you didn't declare the argument type,
something as simple as malloc(42) invokes undefined behavior, since
you're passing an argument of type int.

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.

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
Sep 3 '07 #26
Barry Schwarz <sc******@doezl.netwrites:
On Sat, 01 Sep 2007 13:06:55 -0700, Fr************@googlemail.com
wrote:
[...]
>>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.)

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Sep 3 '07 #27
Keith Thompson wrote:
>
Barry Schwarz <sc******@doezl.netwrites:
On Sat, 01 Sep 2007 13:06:55 -0700, Fr************@googlemail.com
wrote:
[...]
>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."

--
pete
Sep 3 '07 #28
pete <pf*****@mindspring.comwrites:
Keith Thompson wrote:
>Barry Schwarz <sc******@doezl.netwrites:
On Sat, 01 Sep 2007 13:06:55 -0700, Fr************@googlemail.com
wrote:
[...]
>>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'.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Sep 3 '07 #29

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

4 posts views Thread by Sims | last post: by
17 posts views Thread by lkrubner | last post: by
28 posts views Thread by Benjamin Niemann | last post: by
40 posts views Thread by Edward Elliott | last post: by
reply views Thread by NPC403 | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.