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.
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;
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.
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 */
}
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.