By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
458,192 Members | 1,784 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 458,192 IT Pros & Developers. It's quick & easy.

Inconsistent Program Results

P: n/a
I am learning C, having fun with strings & pointers at the moment!

The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".

Any ideas what's going on?

TIA!
#include <malloc.h>
#include <stdio.h>
#include <memory.h>

#define LEN 1000

void rdinpt();

void main()
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
printf("%s\n", ++restrict);
free(s), s=restrict=0;
return (0);
}

void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
(void) gets(*s);
return(0);
}

Mar 6 '07 #1
Share this Question
Share on Google+
20 Replies


P: n/a
Fr************@googlemail.com said:
I am learning C, having fun with strings & pointers at the moment!

The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".

Any ideas what's going on?

TIA!
#include <malloc.h>
Replace this with <stdlib.hwhich is the standard header you need when
using malloc.
#include <stdio.h>
This is fine.
#include <memory.h>
Replace this with <string.hwhich is the standard header you need when
using strchr.
#define LEN 1000

void rdinpt();
Make this:

void rdinput(char **);

Incidentally, readinput would have been a better name.
>
void main()
Make this:

int main(void)
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
If the malloc fails, s will have the value NULL, which is not legal for
passing to strchr.

If there is no space in the input string, strchr will return NULL, which
you must not pass to printf to match %s, and which you must not
increment.
printf("%s\n", ++restrict);
free(s), s=restrict=0;
This made me reach for the book. Simplify:

free(s);
s = restrict = 0;
return (0);
return 0;

is fine. The return keyword is not a function name.
}

void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
Better:

*s = malloc(LEN * sizeof **s);

The general form is: p = malloc(n * sizeof *p);
(void) gets(*s);
Avoid gets() - it cannot be used safely. Instead, use:

fgets(s, LEN, stdin);
return(0);
Remove this, since rdinpt doesn't return a value.
}
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.
Mar 6 '07 #2

P: n/a
Fr************@googlemail.com wrote:
I am learning C, having fun with strings & pointers at the moment!
The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".
#include <malloc.h>
This is not a standard header. You probably should be include
<stdlib.hinstead.
#include <stdio.h>
#include <memory.h>
Another non-standard header, I have no idea what you would
need that for. Avoid non-standard headers unless you know
why you use them, i.e. if you do something system-specific
- but there's nothing system-specific in your program that
would require this.
#define LEN 1000
void rdinpt();
Why do you lie to the compiler? The function of the same name
you define later takes an argument, a pointer to pointer to
char.
void main()
Under both Windows and Linux main() always returns an int (and
your main function aactually returns an int). And to make it
clear that it doesn't take any arguments better write it as

int main( void ):
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
If there's no space in the string you got then strchr() returns
NULL.
printf("%s\n", ++restrict);
That forces printf() in the case that there was no space in the
string you got to dereference NULL+1 - whatever that may point
to it's rather likely that this isn't any memory you own. And
in that case you have undefined behaviour and everything can
happen. Printing nothing or segfaulting are only two of many
possibilities.

So you need to check the return value of strchr() and only
try to print out something if that wasn't NULL.
free(s), s=restrict=0;
Why use the comma operator instead of a semicolon here? And
since both 's' and 'restrict' are pointers you better make
that clear by assigning NULL instead. The next person having
to read your code will thank you.
return (0);
}
void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
Why the cast to 'char*'? I suspect you put it there to keep
the compiler from complaining about an assignment from an
int to a pointer. But casting is not the solution in that
case, it only makes things worse. You need to include
<stdlib.hto make the compiler aware that malloc() returns
a void pointer and not an int, as it will assume if it does
not know about the return type of malloc(). And, by the way,
the argument to malloc() is of type 'size_t', not unsigned
int. But casting here is also superfluous. If the compiler
knows what kind of argument malloc() expects it will do the
conversion all by itself if necessary (another argument for
including <stdlib.h>).

And if you use malloc() never forget to check its return value.
It can fail, returning NULL, and then you not only have no
memory but if you use this return value your program may crash
or do even worse things.
(void) gets(*s);
And now you used _the_ function you never ever should use.
It is broken by design since if the user enters more than
LEN-1 characters the function will write past the end of
the memory you allocated. So, never ever even think of
using it. Use fgets() instead - there you can tell how
many chars you are prepared to read in.
return(0);
Strange, above you told that the function doesn't return
anyting... And, by the way, you don't need parentheses
around the return value, 'return' isn't a function.
}
If you don't already have ask the compiler to output lots
of warnings (e.g. using at least the options '-W -Wall' if
you use gcc). Try to understand what makes the compiler
complain and correct your program so that it compiles
cleanly. This will teach you a lot about the mistakes you
are making.
Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Mar 6 '07 #3

P: n/a
In article <11**********************@q40g2000cwq.googlegroups .com>,
Fr************@googlemail.com wrote:
I am learning C, having fun with strings & pointers at the moment!

The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".

Any ideas what's going on?
<delurk>
I'll take a crack at this, others might find other problems, though.
>
TIA!
#include <malloc.h>
This is a nonstandard header; malloc is declared in stdlib.h, so you
should include that header.
#include <stdio.h>
#include <memory.h>
Another nonstandard header that doesn't seem to do anything.
>
#define LEN 1000

void rdinpt();
This should be
void rdinpt (char **);
>
void main()
The return type of main is int, so this should be int main (),
or int main (void), most regulars I believe would recommend the second.
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
printf("%s\n", ++restrict);
Here's one problem, check the standard (or the man page), if strchr
doesn't find the character being searched for, it sets the return value
to NULL, as will be the case when you input only one word. You then try
to increment a NULL pointer, that's undefined behavior, so anything can
happen, including doing nothing or segfaulting. You have to guard
against this by making sure that restrict is not NULL, so do something
like:
if (restrict)
{
printf ("%s\n", ++restrict);
}
By the way, restrict is a type qualifier in C, so it's probably not a
good name for a variable.
free(s), s=restrict=0;
If you want to set these pointers to NULL, say so. Although, since you
free s then end the program, it's not necessary.
return (0);
The parentheses around 0 are unnecessary (but not wrong)
}

void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
In C, the cast to char * is unnecessary and can mask failure to #include
<stdlib.h>. Also, LEN will be automatically converted to size_t (I
believe), so the cast to unsigned int is unnecessary. Also, before
using s, you need to make sure that malloc sucessfully allocated the
memory requested, if it didn't, s will be NULL, so test s for NULL, like
I did for restrict above.
(void) gets(*s);
Never use gets; you cannot prevent buffer overflow. Better to use
fgets, or use CBFalconer's ggets (I'm sure he'll be along soon with the
URL for his download page)
return(0);
You declared this function with a return type of void, i.e. no return,
so don't return anything.
}
That's all I can find here (how'd I do?)
<\delurk>
Mar 7 '07 #4

P: n/a
"D. Power" <pow...@pcisys.netwrote:
Francine.Ne...@googlemail.com wrote:
...
char *s, *restrict;
...
By the way, restrict is a type qualifier in C,
It's a qualifier in C99, but not in C90.
so it's probably not a good name for a variable.
It's the fact that it's a keyword in C99 that makes it
unavailable for use as an ordinary identifier (in C99,)
rather than being a type qualifier per se.
free(s), s=restrict=0;

If you want to set these pointers to NULL, say so.
He did. ;-)

It's a style issue, as is using the comma operator rather
than separate statements.

NULL was intended to replace an anachronism, however 0 remains
entrenched in the C language as a null pointer constant. The
fact that C++ encourages 0 instead of the NULL macro also adds
influence on C programmers.

....
void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);

In C, the cast to char * is unnecessary and can mask failure to
#include <stdlib.h>. Also, LEN will be automatically converted to
size_t (I believe),
Only if a prototype is in scope.
so the cast to unsigned int is unnecessary.
It's always unnecessary since size_t needn't be unsigned int.

--
Peter

Mar 7 '07 #5

P: n/a
On Mar 7, 12:32 pm, j...@toerring.de (Jens Thoms Toerring) wrote:
Francine.Ne...@googlemail.com wrote:
void rdinpt();

Why do you lie to the compiler? The function of the same name
you define later takes an argument, a pointer to pointer to
char.
He isn't lying. The above declares a function named 'rdinpt',
returning void, and taking arguments of an unspecified number
and type(s).

Mar 7 '07 #6

P: n/a
Fr************@googlemail.com wrote:
I am learning C, having fun with strings & pointers at the moment!

The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".

Any ideas what's going on?
Yes, you have a very small piece of code with a very large number of
errors. Here is a starting place for you to think about how to write
this program somewhat better:

/*
The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".
*/

#if 0
/* mha: There are no such standard headers as <malloc.hor <memory.h>
*/
#include <malloc.h>
#include <memory.h>
#endif
#include <stdio.h>
#include <string.h /* mha: added for strchr */
#include <stdlib.h /* mha: added for malloc and free (and
exit) */

#define LEN 1000

void rdinpt(char **); /* mha: completed prototype */

/* mha: declaring main with a return type of void is a gross error.
Do not let anyone see code in which you show that you can't get
even the simplest possible C program right. I have fixed this. */
int main(void)
{
char *s, *rstrct; /* mha: "restrict" is a keyword for
C99. Even though you may have a C89
(C90 or C95) compiler, try to keep
your code clean so it will compile
in the current standard version of
C. I have changed the spelling by
removing the vowels. */
printf("Please type something: "); /* mha: just so the user
doesn't invoke the program
and then see the machine
play dead */
fflush(stdout);

rdinpt(&s);
rstrct = strchr(s, ' ');
/* mha: added handling for input lines with no ' ' */
if (rstrct)
printf("%s\n", ++rstrct);
else
printf("The input string \"%s\" contains no space character\n"
"So an attempt to print a string beginning after\n"
"that space would be nonsense.\n", s);
free(s);
return 0; /* mha: this is OK now, but before you
were (incorrectly) claiming that
main returned nothing, but were
trying to return a value */
}

void rdinpt(char **s)
{
char *nl;

/* mha: fixed verbose and error-prone use of malloc */
*s = malloc(LEN);
/* mha: added error check */
if (!*s) {
fprintf(stderr, "malloc failed.\nQuitting ...\n");
exit(EXIT_FAILURE);
}

/* mha: one must be insane to use gets. I have changed this */
if (!fgets(*s, LEN, stdin)) {
fprintf(stderr, "Undiagnosed error on input.\nQuitting ..\n");
free(*s);
exit(EXIT_FAILURE);
}
if ((nl = strchr(*s, '\n')))
*nl = 0;

/* mha: fixed erroneous attempt to return a value from a function
declared not to return anything. */
}

TIA!
#include <malloc.h>
#include <stdio.h>
#include <memory.h>

#define LEN 1000

void rdinpt();

void main()
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
printf("%s\n", ++restrict);
free(s), s=restrict=0;
return (0);
}

void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
(void) gets(*s);
return(0);
}
Mar 7 '07 #7

P: n/a
In article <11*********************@8g2000cwh.googlegroups.co m>,
"Peter Nilsson" <ai***@acay.com.auwrote:
"D. Power" <pow...@pcisys.netwrote:
Francine.Ne...@googlemail.com wrote:
...
char *s, *restrict;
...
By the way, restrict is a type qualifier in C,

It's a qualifier in C99, but not in C90.
I see, thank you for that clarification.
>
so it's probably not a good name for a variable.

It's the fact that it's a keyword in C99 that makes it
unavailable for use as an ordinary identifier (in C99,)
rather than being a type qualifier per se.
free(s), s=restrict=0;

If you want to set these pointers to NULL, say so.

He did. ;-)

It's a style issue, as is using the comma operator rather
than separate statements.
Yes, I should have been clearer on that point.
[snip]
Mar 7 '07 #8

P: n/a
I hadn't expected such detailed replies... it seems that in fact it
was just a silly oversight, and adding if(restrict) above the printf
statement was all I needed to do to get things working, but I'm very
grateful to those who made detailed comments on other parts of my code
too. I'm definitely going to take them on board - I'm trying to learn
C in an accelerated way at the moment, and these concentrated comments
are really helpful.
#include <malloc.h>

Replace this with <stdlib.hwhich is the standard header you need when
using malloc.
I think what happens is that stdlib is modularized, and all the
allocation functions are in malloc.h, so just including this avoids
bloating the compiled executable with extraneous cruft from stdlib.
#include <memory.h>

Replace this with <string.hwhich is the standard header you need when
using strchr.
OK, though on my system memory.h just does #include <string.hanyway.
void rdinpt();

Make this:

void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?
Incidentally, readinput would have been a better name.
Sounds like more typing :(
void main()

Make this:

int main(void)
What's the advantage to this? In my textbook, main only returns int if
the return value is significant for something (e.g. if main is called
recursively).
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');

If the malloc fails, s will have the value NULL, which is not legal for
passing to strchr.

If there is no space in the input string, strchr will return NULL, which
you must not pass to printf to match %s, and which you must not
increment.
Understood - thanks.
free(s), s=restrict=0;

This made me reach for the book. Simplify:

free(s);
s = restrict = 0;
I think the comma is useful because it shows that there's one
conceptual operation (free a pointer and remove bogus references),
which gets broken up if you use two separate statements.
*s=(char *) malloc( (unsigned int) LEN);

Better:

*s = malloc(LEN * sizeof **s);
Well, OK, if you know that char * and void * have the same internal
representation, but as I understand it you'd still need to typecase if
*s was an int * for the sake of alignment.
(void) gets(*s);

Avoid gets() - it cannot be used safely. Instead, use:
I read the man page for gets, and it does seem that in production code
it should be avoided. But for a little program, 1000 bytes is already
a conservative buffer size.
return(0);

Remove this, since rdinpt doesn't return a value.
Thanks, I always forget whether to return() or return(0) at the end of
a void function.
}

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999http://www.cpax.org.uk
email: rjh at the above domain, - www.
Mar 7 '07 #9

P: n/a
Fr************@googlemail.com wrote:
I think what happens is that stdlib is modularized, and all the
allocation functions are in malloc.h, so just including this avoids
bloating the compiled executable with extraneous cruft from stdlib.
If that works for your platform, fine, but be aware that malloc.h is
NOT a standard header, whereas stdlib.h is guaranteed on all hosted
implementations.
void rdinpt();
void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?
It's not obligatory at all, but the first is not a prototype;
therefore the second form is preferable.
void main()
int main(void)
What's the advantage to this? In my textbook, main only returns int if
the return value is significant for something (e.g. if main is called
recursively).
The "advantage" is that int main( void ) (or its cousin) is required
by the Standard. http://c-faq.com/ansi/maindecl.html
I read the man page for gets, and it does seem that in production code
it should be avoided. But for a little program, 1000 bytes is already
a conservative buffer size.
It's worth the small amount of extra effort to avoid using gets() for
anything, even small toy programs. gets() really is that bad.

--
C. Benson Manica | I *should* know what I'm talking about - if I
cbmanica(at)gmail.com | don't, I need to know. Flames welcome.
Mar 7 '07 #10

P: n/a
Fr************@googlemail.com writes:
I hadn't expected such detailed replies... it seems that in fact it
was just a silly oversight, and adding if(restrict) above the printf
statement was all I needed to do to get things working, but I'm very
grateful to those who made detailed comments on other parts of my code
too. I'm definitely going to take them on board - I'm trying to learn
C in an accelerated way at the moment, and these concentrated comments
are really helpful.
#include <malloc.h>

Replace this with <stdlib.hwhich is the standard header you need when
using malloc.

I think what happens is that stdlib is modularized, and all the
allocation functions are in malloc.h, so just including this avoids
bloating the compiled executable with extraneous cruft from stdlib.
<malloc.his not a standard header; you can't assume that it will
exist at all in other implementations. By using <stdlib.hrather
than <malloc.h>, you gain portability (and make the program easier to
read for users who are familiar with standard C), and you lose
nothing. The standard headers will contain only declarations;
including one of them might trivially slow down compilation, but it
won't cause any bloat in your executable.
#include <memory.h>

Replace this with <string.hwhich is the standard header you need when
using strchr.

OK, though on my system memory.h just does #include <string.hanyway.
And on another system, <memory.hmight not exist.
void rdinpt();

Make this:

void rdinput(char **);

I don't think it's obligatory to list the parameters yet is it?
It's not obligatory, but you should *always* do it anyway; there's no
good reason not to. Using a prototype (i.e., specifying the types of
the parameters) allows the compiler to detect certain errors, and in
many cases to adjust the argument via an implicit conversion, and
costs you nothing.
>Incidentally, readinput would have been a better name.

Sounds like more typing :(
More typing, and easier reading. How many times to you type the name?
How many times will you and others read it?

D y lk rdng txt tht's wrttn lk ths? Was the time I saved typing that
sentence worth the extra time it took you to figure out what it means?
void main()

Make this:

int main(void)

What's the advantage to this? In my textbook, main only returns int if
the return value is significant for something (e.g. if main is called
recursively).
Get a better textbook.

main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.

[snip]
free(s), s=restrict=0;

This made me reach for the book. Simplify:

free(s);
s = restrict = 0;

I think the comma is useful because it shows that there's one
conceptual operation (free a pointer and remove bogus references),
which gets broken up if you use two separate statements.
Using the comma operator here makes it more difficult to read.
*s=(char *) malloc( (unsigned int) LEN);

Better:

*s = malloc(LEN * sizeof **s);

Well, OK, if you know that char * and void * have the same internal
representation, but as I understand it you'd still need to typecase if
*s was an int * for the sake of alignment.
No no no no no!

malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.

(Well, very nearly so. If you have a good reason to want to compile
your code either as C or as C++, the cast is necessary. But I've
encountered a grand total of two people here who have a good reason to
do this; you're not one of them (and neither am I).)
(void) gets(*s);

Avoid gets() - it cannot be used safely. Instead, use:

I read the man page for gets, and it does seem that in production code
it should be avoided. But for a little program, 1000 bytes is already
a conservative buffer size.
gets() can *never* be used safely. If you provide a 1000-byte buffer,
how do you prevent the user of your program from giving you 1100 bytes
of input?

Use fgets() (and read its documentation to understand how it deals
with newline characters).
return(0);

Remove this, since rdinpt doesn't return a value.

Thanks, I always forget whether to return() or return(0) at the end of
a void function.
"return();" is a syntax error. "return(0);" or "return 0;" is
incorrect in a void function. "return;" is unnecessary unless you're
terminating the function early.

And for non-void functions, you don't need parentheses on a return
statement.

If your function returns void, and you don't need to terminate it
early, you don't need a return statement at all; falling off the end
of the function (reaching the closing "}") is exactly equivalent to
executing a "return;" statement.
}
--
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"
Mar 7 '07 #11

P: n/a
On Mar 6, 4:41 pm, Francine.Ne...@googlemail.com wrote:
I am learning C, having fun with strings & pointers at the moment!

The following program is my solution to an exercise to take an input,
strip the first word, and output the rest. It works fine when you give
it 2 or more words, but when there's only 1 word the results vary
depending on whether it's on Windows or Linux: under MSVC it displays
no output (as it should); under gcc/Linux it instead gives
"Segmentation fault".

Any ideas what's going on?

TIA!

#include <malloc.h>
Try to use standard headers whenever possible, at least when posting
to this group; those of us not familiar with your particular
development environment will have an easier time understanding your
code. In this case, use stdlib.h.
#include <stdio.h>
#include <memory.h>
Same as above. This is a non-standard header; I'm not sure what it's
supposed to do.
#define LEN 1000

void rdinpt();
void rdinput(char **);

It's best to declare functions using prototype syntax (actually, it's
best to define functions before they are first used, but that's not
always possible). Prototypes allow the compiler to catch mistakes in
passing the wrong number or types of parameters, making the
development process a little more robust.
void main()
The standard, well-supported definitions for main are

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

Some implementations may extend these definitions, but in general it's
best to stick to one of those two. Unless your implementation
specifically says that void main() is supported, using it will invoke
undefined behavior, which may or may not cause problems during program
execution; the program may even fail to load on certain platforms.

A *lot* of C books and tutorials make this mistake. That's because a
*lot* of C books and tutorials are crap.
{
char *s, *restrict;
rdinpt(&s);
restrict=strchr(s,' ');
And here is your real problem. If you've only entered one word, then
there should be no ' ' character in s. If there is no ' ' character
in s, then strchr returns NULL. Attempting to do anything with a NULL
pointer beyond using it in a comparison (such as incrementing it and
attempting to read the contents of it) invokes undefined behavior.

And, you can see the problem with undefined behavior; the results are
inconsistent across platforms. At least the Linux platform lets you
know there's a problem; the Windows platform lulls you into a false
sense of security that the program is running correctly, which is far
more dangerous. I've seen hundreds of situations like this, where the
code appears to be running fine until it gets out into the field, and
a customer feeds it just the right combination of data to cause
unimaginable mayhem, and the next thing you know tech support is
looking for your head because a customer just trashed all their
data.

At the very least, you need to test that restrict is not NULL before
attempting to do anything with it. Also, I think restrict is a
reserved word in the latest C standard (C99), so it's not the best
choice for a variable name.

if (restrict)
printf("%s\n", ++restrict);
free(s), s=restrict=0;
Bad style. Make these two separate statements.

free(s);
s = restrict = 0;
return (0);
You've typed main to return void (erroneously), so returning a value
doesn't make much sense. However, you do need to return a value in a
properly typed main function. Since return is a statement, not a
function, the parentheses around the 0 are unnecessary. IOW, the
grammar for a return is

return EXPR ;

not

return ( EXPR ) ;

The extra parens don't hurt anything; they're just not necessary.
>
}

void rdinpt(char **s)
{
*s=(char *) malloc( (unsigned int) LEN);
Don't cast the result of malloc(). First of all, you don't need to:
objects of type void* are implicitly converted to other object pointer
types (if you get a diagnostic on this, then you're either using a
*very* old compiler (pre-ANSI), or you're actually compiling this code
as C++). Secondly, the cast may mask a warning in case you don't have
a prototype for malloc() in scope. Without a prototype, malloc() will
be implicitly declared to return an int; this would cause a type
mismatch diagnostic to be generated by the compiler, but the presence
of the cast will suppress that diagnostic.

However, if you are using a *very* old implementation of C (pre-ANSI,
C89), malloc() may be defined as returning char*, in which case the
cast *is* necessary.

The common convention for a malloc() call is

p = malloc(n * sizeof *p);

for a given pointer p and n number of elements.

*ALWAYS* check the result of malloc() before attempting to use the
allocated object.
(void) gets(*s);
Don't use gets(). Ever. It *will* introduce a point of failure in
your program. Don't even use it in toy code. It's evil. Use fgets()
instead:

if(!fgets(s, LEN, stdin))
{
if (feof(stdin))
/* EOF detected on standard input */
else
/* Error occured while reading standard input */
}
return(0);
Same issue as above; since this function is typed to return void, you
don't return a value, so lose the (0). In fact, you could omit the
return statement entirely.
Mar 7 '07 #12

P: n/a
The standard headers will contain only declarations;
including one of them might trivially slow down compilation, but it
won't cause any bloat in your executable.
OK, that's interesting, thanks.
void rdinpt();
Make this:
void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?

It's not obligatory, but you should *always* do it anyway; there's no
good reason not to. Using a prototype (i.e., specifying the types of
the parameters) allows the compiler to detect certain errors, and in
many cases to adjust the argument via an implicit conversion, and
costs you nothing.
Surely it makes the code harder to maintain? If I change the parameter
types of rdinpt then I'd have to go back and change it consistently
somewhere else too. As I understand it, the main point of putting in
this declaration is so that I call rdinpt from a function that occurs
before its body in the source file, and leaving the parameters
flexible still achieves this.
Sounds like more typing :(

More typing, and easier reading. How many times to you type the name?
How many times will you and others read it?
Well, with things like strspn, atoi and setjmp in the standard
library, brevity of names seems to have good precedents in C.
Get a better textbook.

main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.
This seems quite a basic point, and I doubt a textbook would get it
wrong.

It costs some thought - if you don't have anything useful to return
from main, what int should you return? 0? 1? 19223?
malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.
OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.
Thanks, I always forget whether to return() or return(0) at the end of
a void function.

"return();" is a syntax error. "return(0);" or "return 0;" is
incorrect in a void function. "return;" is unnecessary unless you're
terminating the function early.
Hah! Now I remember exactly what happened. I started with return(),
got an error, and assumed I had to return(0) instead - this only gave
a warning, but gcc is extremely picky and produces lots of warnings,
so I tend to ignore them. Why is return() a syntax error? That seems
pretty inconsistent with the syntax for calling a non-built-in
function with no arguments.
And for non-void functions, you don't need parentheses on a return
statement.

If your function returns void, and you don't need to terminate it
early, you don't need a return statement at all; falling off the end
of the function (reaching the closing "}") is exactly equivalent to
executing a "return;" statement.
}

--
Keith Thompson (The_Other_Keith) k...@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"
Mar 8 '07 #13

P: n/a
Fr************@googlemail.com said:
I hadn't expected such detailed replies...
That wasn't in fact a detailed reply. A detailed reply would have taken
me longer than I had available.
it seems that in fact it
was just a silly oversight, and adding if(restrict) above the printf
statement was all I needed to do to get things working,
No, that was merely all you had to do to get enough life into your car
to get it to the garage. Its wheels are still the wrong shape, and it
still has bits of dirt in the fuel line, and it still has defective
brakes.
#include <malloc.h>

Replace this with <stdlib.hwhich is the standard header you need
when using malloc.

I think what happens is that stdlib is modularized, and all the
allocation functions are in malloc.h, so just including this avoids
bloating the compiled executable with extraneous cruft from stdlib.
Not so. There are no functions in malloc.h (if it is a properly written
header), just function declarations, which should not take any space in
the binary if your compiler knows its stuff - and linkers do not link
arbitrary junk from the standard library, only the stuff that is
actually needed. Of course, we can't be all that specific about
malloc.h since there is actually no such header from our perspective.
All hosted C implementations *must* provide stdlib.h, but none of them
need provide malloc.h, and even those that do are not constrained as to
what that header contains.
#include <memory.h>

Replace this with <string.hwhich is the standard header you need
when using strchr.

OK, though on my system memory.h just does #include <string.hanyway.
By using <memory.hinstead of <string.hyou are restricting your
program to use only on those systems where memory.h exists and does
what yours does.

When you're asking for help, it's better not to restrict your program so
much that it won't compile on the systems of those people who might
otherwise have taken the trouble to help you.
>
void rdinpt();

Make this:

void rdinput(char **);

I don't think it's obligatory to list the parameters yet is it?
No, you're right, it isn't (and I didn't).
>
>Incidentally, readinput would have been a better name.

Sounds like more typing :(
Following that advice religiously, I present your original program
again.

#include <malloc.h>
#include <stdio.h>
#include <memory.h>
#define L 1000
void r();void main(){char*s,*t;r(&s);t=strchr(s,' ');printf("%s\n",++t);
free(s),s=t=0;return(0);}void r(char**s){*s=(char*)malloc((unsigned int)
L);(void)gets(*s);return(0);}

That's less typing - but it's much harder reading and fixing, isn't it?

Write your programs in such a way that they are easy to understand,
because a program you don't understand is a buggy program.
void main()

Make this:

int main(void)

What's the advantage to this?
main is an interface between you and the system. Whenever you are in
control of both halves of an interface, you get to choose the
interface. If you suddenly decide you don't like it, you get to change
it - but of course you have to change the code on both sides.

Unless you fancy rewriting and recompiling and retesting your compiler
every time you get bored with int main, I suggest you stick to the
standard, well-defined, predictable, portable, easy interface spec
where main returns int.
In my textbook, main only returns int if
the return value is significant for something (e.g. if main is called
recursively).
Then your text book is wrong.
free(s), s=restrict=0;

This made me reach for the book. Simplify:

free(s);
s = restrict = 0;

I think the comma is useful because it shows that there's one
conceptual operation (free a pointer and remove bogus references),
which gets broken up if you use two separate statements.
Well, you managed to surprise me. I didn't think there could be any
justification for what you were doing, but you've come up with one that
made me stop and think. Okay, I'm not going to yell at you for that
one. It's not something I'd do, but I can see why you do it.

*s=(char *) malloc( (unsigned int) LEN);

Better:

*s = malloc(LEN * sizeof **s);

Well, OK, if you know that char * and void * have the same internal
representation,
Whilst that is true, it is irrelevant.
but as I understand it you'd still need to typecase if
*s was an int * for the sake of alignment.
Not at all. In general, for any object type T:

T *p = malloc(n * sizeof *p);
(void) gets(*s);

Avoid gets() - it cannot be used safely. Instead, use:

I read the man page for gets, and it does seem that in production code
it should be avoided. But for a little program, 1000 bytes is already
a conservative buffer size.
Not to the guy who has 1000 bytes of yawn-yawn and a few dozen bytes of
machine code waiting to sit on your stack to get root access to your
machine.
return(0);

Remove this, since rdinpt doesn't return a value.

Thanks, I always forget whether to return() or return(0) at the end of
a void function.
Neither. Use:

return;

at the end of a void function (optionally - it can be omitted), and:

return value_of_appropriate_type;

at the end of a non-void function (non-optionally).

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.
Mar 8 '07 #14

P: n/a
Fr************@googlemail.com said:
void rdinpt();
>Make this:
> void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?

It's not obligatory, but you should *always* do it anyway; there's no
good reason not to. Using a prototype (i.e., specifying the types of
the parameters) allows the compiler to detect certain errors, and in
many cases to adjust the argument via an implicit conversion, and
costs you nothing.

Surely it makes the code harder to maintain?
No, easier.
If I change the parameter
types of rdinpt then I'd have to go back and change it consistently
somewhere else too.
Yes, the program will need changing in all the places where you called
rdinput(). If you don't list the parameter types in the declaration,
the compiler won't be able to track these places down quickly for you.
As I understand it, the main point of putting in
this declaration is so that I call rdinpt from a function that occurs
before its body in the source file, and leaving the parameters
flexible still achieves this.
It does, but the whole point of declaring stuff is so that the compiler
can do a basic amount of type-checking for you.
>
Sounds like more typing :(

More typing, and easier reading. How many times to you type the
name? How many times will you and others read it?

Well, with things like strspn, atoi and setjmp in the standard
library, brevity of names seems to have good precedents in C.
No - *bad* precedents. But at least everyone knows what those things do.
The object and purpose of the rdinpt function is not hanging on the
lips of every C student, so they are less likely to know what it means
than they are to know what, say, strspn means. Good name choice is an
important part of programming, but it seems to be a part that wasn't as
well-understood when functions like strspn were being coined.
>Get a better textbook.

main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.

This seems quite a basic point, and I doubt a textbook would get it
wrong.
I've written a textbook (no, really I have) in which I state that main
returns int, always always always, in ALL hosted implementations. Since
by your argument textbooks never get stuff wrong, I must be right, yes?

In fact, it's because textbooks /do/ get it wrong that we need a
Standard.
It costs some thought - if you don't have anything useful to return
from main, what int should you return? 0? 1? 19223?
You always have something useful to return from main() - the success or
otherwise of the program. If the program succeeded, either return 0 or
#include <stdlib.hand return EXIT_SUCCESS (these two are synonymous -
they both mean "yippee, it worked". If, however, it failed, then
#include <stdlib.hand return EXIT_FAILURE. These are the only three
values you can /portably/ return from main.
>malloc(), if it succeeds, returns a pointer (of type void*) to a
chunk of memory that's guaranteed to be properly aligned for any data
type. A void* pointer can be implicitly converted to any
pointer-to-object
type. The cast is absolutely unnecessary.

OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.
So is driving with one hand on the wheel and the other wrapped round a
mobile phone. Doesn't make it right.

[...] gcc is extremely picky and produces lots of warnings,
so I tend to ignore them.
Those warnings are there for your benefit. Instead of ignoring them,
learn what they mean, understand them, and then fix your code so that
it doesn't produce them (without breaking it in other ways).
Why is return() a syntax error? That seems
pretty inconsistent with the syntax for calling a non-built-in
function with no arguments.
return isn't a function.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.
Mar 8 '07 #15

P: n/a
In article <11**********************@p10g2000cwp.googlegroups .com>
<Fr************@googlemail.comwrote:
>Surely [using prototypes] makes the code harder to maintain?
Well, yes; but also no.

In particular, if you have a prototype declaration for a function,
and you change the function to take different parameters (but no
change to its return type), you must find and update the prototype.
(The parenthetic remark is to distinguish this case from the one
where you *do* change the return type; in this case, you must
edit any declaration regardless of whether it is also a prototype.)

This is "new work" that you did not have to do if you did not use
a prototype.

On the obverse, however, if you change a function to take different
parameters, you must find and update all calls to that function.
This maintenance *task* is unchanged whether or not you use a
prototype -- but if you *do* use a prototype, any calls that you
miss will [1] draw a diagnostic from any correct [2] C compiler.
This makes the actual maintenance easier: now you can make any
changes you know you need to make, then start a compilation, and
make any additional changes the compiler points out.

There are languages that avoid the need for redundant declarations
like prototypes. Compilers for these languages (if they are in
fact compiled) automatically record the actual type of the actual
definition, and automatically compare calls. In effect, these
compilers build prototypes for you. C is not such a language, but
if you use prototypes consistently, you can get most of this benefit,
at a relatively low cost.

-----
[1] Provided the prototype is in scope at the point of the call,
anyway. Better compilers have flags to warn you if you make a call
without a prototype in scope, so that you can be sure that all
calls have a prototype. If you always put such prototypes in a
"safe" location -- such as a header file that you also include in
the code that defines the function -- then you get consistent and
"safe" checking. If you scatter multiple copies of the prototype
in multiple separate files, however, and/or fail to have an
appropriate prototype in place at the point of the call, you lose
the benefit. This is where those Other Languages with "automatic"
prototyping seem superior, at least up until you have to write the
compiler for one. :-)

[2] "Correct" is kind of a weasel-word for "conforming". A lot of
modern C compilers will warn even in non-conforming modes, such as
their default modes, so "conforming" is not quite the right adjective
anyway.
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (4039.22'N, 11150.29'W) +1 801 277 2603
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.
Mar 8 '07 #16

P: n/a
Fr************@googlemail.com writes:
>The standard headers will contain only declarations;
including one of them might trivially slow down compilation, but it
won't cause any bloat in your executable.

OK, that's interesting, thanks.
I wrote the above (starting with "The standard headers"). Please
don't delete attribution lines.
void rdinpt();
>Make this:
> void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?

It's not obligatory, but you should *always* do it anyway; there's no
good reason not to. Using a prototype (i.e., specifying the types of
the parameters) allows the compiler to detect certain errors, and in
many cases to adjust the argument via an implicit conversion, and
costs you nothing.

Surely it makes the code harder to maintain? If I change the parameter
types of rdinpt then I'd have to go back and change it consistently
somewhere else too. As I understand it, the main point of putting in
this declaration is so that I call rdinpt from a function that occurs
before its body in the source file, and leaving the parameters
flexible still achieves this.
If you change the parameter types, you have to change all the calls as
well. Leaving the parameter types out doesn't give you any real
flexibility; it just makes errors more difficult to detect.
Sounds like more typing :(

More typing, and easier reading. How many times to you type the name?
How many times will you and others read it?

Well, with things like strspn, atoi and setjmp in the standard
library, brevity of names seems to have good precedents in C.
Sure, but we know what "atoi" means, and if we don't, we can look it
up in the standard. We don't have that advantage when we encounter
the name "rdinpt".

When the names of the functions in the standard library were chosen,
some linkers limited external identifiers to just 6 significant
characters, and hardware (teletypes) was such that long identifiers
were physically more difficult to type (or so I've heard; it was
before my time). We're no longer subject to those limitations.
>Get a better textbook.

main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.

This seems quite a basic point, and I doubt a textbook would get it
wrong.
I admit it's surprising, but it happens to be true. "void main()" is
incorrect, and many textbooks really do get it wrong. See questions
11.12a through 11.15 in the comp.lang.c FAQ, <http://www.c-faq.com/>.
And if that doesn't convince you, the latest version of the C standard
is available at
<http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf>.
It costs some thought - if you don't have anything useful to return
from main, what int should you return? 0? 1? 19223?
0.

The only portable values you can return from main() are 0,
EXIT_SUCCESS, and EXIT_FAILURE; the latter are macros defined in
<stdlib.h>.

Yes, there are things you have to remember.
>malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.

OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.
You see a lot of bad code.
Thanks, I always forget whether to return() or return(0) at the end of
a void function.

"return();" is a syntax error. "return(0);" or "return 0;" is
incorrect in a void function. "return;" is unnecessary unless you're
terminating the function early.

Hah! Now I remember exactly what happened. I started with return(),
got an error, and assumed I had to return(0) instead - this only gave
a warning, but gcc is extremely picky and produces lots of warnings,
so I tend to ignore them. Why is return() a syntax error? That seems
pretty inconsistent with the syntax for calling a non-built-in
function with no arguments.
"return" is *not* a function, it's a keyword. A return statement
isn't a function call; it's a special kind of statement with its own
syntax.

You're cheating yourself by ignoring warning messages. gcc can be
picky, but most of its warnings are valid. If you have a question
about a particular warning message, feel free to ask here.

--
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"
Mar 8 '07 #17

P: n/a
On Thu, 08 Mar 2007 08:29:46 +0000, Richard Heathfield
<rj*@see.sig.invalidwrote:
>Fr************@googlemail.com said:
[snip]
>No - *bad* precedents. But at least everyone knows what those things do.
The object and purpose of the rdinpt function is not hanging on the
lips of every C student, so they are less likely to know what it means
than they are to know what, say, strspn means. Good name choice is an
important part of programming, but it seems to be a part that wasn't as
well-understood when functions like strspn were being coined.
And the object and purpose of the readinput function is not
necessarily hanging on the lips of every C student, especially those
who play Scrabble and wonder if it means "re-address input" or "read
in put" or "readin' put". Scrabble players tend to have some wild
thoughts. It's best to make it unambiguous and name it read_input.

Regards
--
jay
Mar 8 '07 #18

P: n/a
On Mar 8, 2:08 am, Francine.Ne...@googlemail.com wrote:
The standard headers will contain only declarations;
including one of them might trivially slow down compilation, but it
won't cause any bloat in your executable.

OK, that's interesting, thanks.
void rdinpt();
>Make this:
> void rdinput(char **);
I don't think it's obligatory to list the parameters yet is it?
It's not obligatory, but you should *always* do it anyway; there's no
good reason not to. Using a prototype (i.e., specifying the types of
the parameters) allows the compiler to detect certain errors, and in
many cases to adjust the argument via an implicit conversion, and
costs you nothing.

Surely it makes the code harder to maintain?
In the end, maintenance is actually easier because you have more
robust error checking (maintenance is more than just editing source
text). You find more errors at compile time, saving you headaches
later on in the testing phase.
If I change the parameter
types of rdinpt then I'd have to go back and change it consistently
somewhere else too. As I understand it, the main point of putting in
this declaration is so that I call rdinpt from a function that occurs
before its body in the source file, and leaving the parameters
flexible still achieves this.
In my experience, you make life easier on yourself by defining
functions before they're used (IOW, you can put the body of the
rdinput() function before main(); that way you don't need a separate
declaration). If you need to call a function from multiple source
files, create a header file that contains a prototype declaration for
that function, and #include it where necessary. That way you only
need to make the prototype change in two places (the actual function
definition, and the header file). Of course you need to change all
the function calls too.
>
Sounds like more typing :(
More typing, and easier reading. How many times to you type the name?
How many times will you and others read it?

Well, with things like strspn, atoi and setjmp in the standard
library, brevity of names seems to have good precedents in C.
Remember that C is a product of the early '70s, and brevity was
necessitated by the limitations of the environment. That's not the
case anymore, and there's no good reason not to use descriptive names.

Trust me, function names that read like indigestion noises are not a
good thing when looking at unfamiliar code (or code that you wrote 6
months ago and haven't looked at since). I have a C reference manual
handy for when I don't remember what grplphmp() does; do you provide
similar documentation?

The gains of using meaningful, descriptive, plain-old-English names
far outweigh the minimal pain of typing them. Remember, the
computer doesn't care whether you call it rdinput or ReadInput, but
anyone trying to read and understand your code will appreciate the
latter over the former.
Get a better textbook.
main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.

This seems quite a basic point, and I doubt a textbook would get it
wrong.
You'd be surprised. The sad fact is that 90% of the books and
websites on C are *crap*. A distressingly large number of them get
basic concepts wrong, and many of them teach bad habits. There are
maybe 5 or 6 references that I consider authoritative.

Herb Schildt proved that you don't have to know what you're talking
about to be a successful author of C references.
>
It costs some thought - if you don't have anything useful to return
from main, what int should you return? 0? 1? 19223?
0 or EXIT_SUCCESS.
>
malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.

OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.
<reiterates prior point about 90% of C references being crap>

Very old implementations of C (pre-ANSI, C89) define malloc() as
returning char*, and in those implementations the cast *was*
necessary, since you can't implicitly convert a char* to another
object pointer type. It could be that the author of your text never
made it past 1985; however, just because he's living in the past
doesn't mean you should too.
Thanks, I always forget whether to return() or return(0) at the end of
a void function.
"return();" is a syntax error. "return(0);" or "return 0;" is
incorrect in a void function. "return;" is unnecessary unless you're
terminating the function early.

Hah! Now I remember exactly what happened. I started with return(),
got an error, and assumed I had to return(0) instead - this only gave
a warning, but gcc is extremely picky and produces lots of warnings,
so I tend to ignore them.
DON'T IGNORE WARNINGS. They're there for a reason. They're telling
you that you've done something that, while not technically illegal, is
ill-advised, and may cause problems at runtime.
Why is return() a syntax error? That seems
pretty inconsistent with the syntax for calling a non-built-in
function with no arguments.
Because return isn't a function; it's a statement. It doesn't have a
parameter list. It can optionally be followed by an expression, and
that expression may be surrounded by parens.

return (0)

is the same as

return 0

because the expressions (0) and 0 evaluate to the same thing.
However, () is not a valid expression, hence the syntax error.

If you're getting all of this from your textbook, throw it in the
trash RIGHT NOW and get something authoritative ("The C Programming
Language", 2nd ed., Kernighan & Ritchie, is about as authoritative as
it gets; a good companion is "C: A Reference Manual", 5th ed.,
Harbison & Steele).

Mar 8 '07 #19

P: n/a
In article <11*********************@s48g2000cws.googlegroups. com>,
John Bode <jo*******@my-deja.comwrote:
>On Mar 8, 2:08 am, Francine.Ne...@googlemail.com wrote:
malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.

OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.

<reiterates prior point about 90% of C references being crap>

Very old implementations of C (pre-ANSI, C89) define malloc() as
returning char*, and in those implementations the cast *was*
necessary, since you can't implicitly convert a char* to another
object pointer type. It could be that the author of your text never
made it past 1985; however, just because he's living in the past
doesn't mean you should too.
[...]
>If you're getting all of this from your textbook, throw it in the
trash RIGHT NOW and get something authoritative ("The C Programming
Language", 2nd ed., Kernighan & Ritchie, is about as authoritative as
it gets; a good companion is "C: A Reference Manual", 5th ed.,
Harbison & Steele).
It's worth noting that K&R2 does tell you to cast malloc.

They have, however, fixed that in the errata (which, for any technical
book, is the first thing you should go looking for after you buy the
book).
They also at least had a good reason - void * was new at the time they
wrote the book, so they were used to casting the char * that came
from malloc, and they tested their code with a C++ compiler (which
also requires the cast) because of a lack of C compilers with the
new-at-the-time prototype checking and other features.

Any C book written since, oh, around 1992 that casts malloc is Wrong
With No Good Reason; the authors should have had a compiler that didn't
require the cast, though they at least may have had the excuse of old
habits to fall back on. *Any* book that declares void main is Wrong
With No Excuse, since it was never necessary or even legal (void was
introduced in C89, which specifically states that main must return int).
dave

--
Dave Vandervies dj******@csclub.uwaterloo.ca
Two glaring problems and one subtle error is a better-than-average
outcome in these parts.
--Eric Sosman in comp.lang.c
Mar 8 '07 #20

P: n/a
On Thu, 8 Mar 2007 02:08:47 -0500, Fr************@googlemail.com wrote
(in article <11**********************@p10g2000cwp.googlegroups .com>):
>Get a better textbook.

main() returns int. Some compilers may allow it to be declared to
return void, but there's no advantage in using that. By declaring
main to return int, you gain portability, and again, it costs you
*nothing*.

This seems quite a basic point, and I doubt a textbook would get it
wrong.
You'd be amazed. There are dozens of books that get it wrong, and
about half of them are written by Herbert Schildt. If you have a book
written by him, return it if you can, otherwise use it to start a fire.
It costs some thought - if you don't have anything useful to return
from main, what int should you return? 0? 1? 19223?
The standard being the ultimate textbook on such things, you return 0,
or EXIT_SUCCESS in such cases.
>malloc(), if it succeeds, returns a pointer (of type void*) to a chunk
of memory that's guaranteed to be properly aligned for any data type.
A void* pointer can be implicitly converted to any pointer-to-object
type. The cast is absolutely unnecessary.

OK then, I'll try to remember not to typecast malloc in future, though
again it seems very common in all the code I've seen.
Bear in mind that if you are just learning C, you haven't seen much
code yet, and those replying to you and by and large people that have
been looking at both good and bad examples of C for years, in some
cases decades. You were smart enough to find a newsgroup populated by
a core group of extremely competent C programmers. Be smart enough to
listen to their advice as well.
>>Thanks, I always forget whether to return() or return(0) at the end of
a void function.

"return();" is a syntax error. "return(0);" or "return 0;" is
incorrect in a void function. "return;" is unnecessary unless you're
terminating the function early.

Hah! Now I remember exactly what happened. I started with return(),
got an error, and assumed I had to return(0) instead - this only gave
a warning, but gcc is extremely picky and produces lots of warnings,
so I tend to ignore them.
*sigh* Wrong attitude. gcc is extremely helpful, in that it produces
a lot of warnings that point out problems in your code, which might
/accidentally/ work on a given day, or a given platform, but are just
likely to not work today, or tomorrow, or when the boss comes along for
that all-important demo. Don't ignore them, understand them so that
you can avoid them in future code.
Why is return() a syntax error? That seems
pretty inconsistent with the syntax for calling a non-built-in
function with no arguments.
What's wrong with:

return;

???
Why should the way you call a function have any commonality of syntax
with the way you return from one?
--
Randy Howard (2reply remove FOOBAR)
"The power of accurate observation is called cynicism by those
who have not got it." - George Bernard Shaw

Mar 18 '07 #21

This discussion thread is closed

Replies have been disabled for this discussion.