struct stat *stbuf seg fault

[David Farning]

I am new to c and not yet sure of the boundary between c and it's
implementations. So please redirect me if this should be asked elsewhere.

I am working on a function to determine if a directory exists. The idea
comes from a snippet I found in a samba source file.

bool dirExist(char *dname)
{
struct stat *stbuf;
//stbuf = (struct stat*)malloc(sizeof(struct stat));
bool ret;

if (stat(dname,stbuf) != 0)
return(False);

ret = S_ISDIR(stbuf->st_mode);
if(!ret)
errno = ENOTDIR;
printf("%s\n",ret);
return ret;
}

It seems to agree with page 180 of K&R2.

When passed a none existant directory name, it correctly reports a failure.
Yet, when passed a existant dir it segfaults at

stat(dname,stbuf)

Mallocing stbuf was just something I have been trying--didn't fix anything.

Thanks
Dave Farning

 

[Charles Harrison Caudill]

I took a literary license and did some critiquing(sp?) of the code.

The direct answer to your question is marked.

bool dirExist(char *dname)
{
struct stat *stbuf;

declaring the struct in this manner will *not* make space on the stack
for the variable. Another way to do this (if you don't like doing
dynamic memory management for structs you know you'll need but don't want to
deal with the &'s all over the place) is:

struct stat _stbuf;
struct stat *stbuf = &_stbuf;

Either the program overflows the stack (and you should have a routine
registered to handle this event) or it works.

I also feel it's a bad idea to declare variables w/o initializing them because
a comparison to NULL later on might or might not reveal it as a useless
pointer. Typically, NULL is 0, and things are *magically* zero out of random
luck, but that's a combination of implementation definitions and luck.

---------------->ANSWER<--------------

The way you have it done as is, space is never being allocated for the buffer.
Consequently when the OS tries to find the file and fails, things go as
planned, but when it succeeds in finding the file, it tries to write to a junk
pointer, and thus segfaults.

//stbuf = (struct stat*)malloc(sizeof(struct stat));

i'm pretty sure // is illegal in ansi 89, but ok in 99

casting the return value of malloc is *bad*. Doing that will cause the
compiler not to complain that you forgot to include stdlib.

bool ret;

if (stat(dname,stbuf) != 0)
return(False);

ret = S_ISDIR(stbuf->st_mode);
if(!ret)
errno = ENOTDIR;
printf("%s\n",ret);
return ret;
}

I'm not sure what editor you're using, but i'm pretty sure that industry
standard is two spaces, and a naming convention of lower case and _'s instead
of mixed case.

Mallocing stbuf was just something I have been trying--didn't fix anything.

Not really sure. I tried test code with the following results:

my way: worked
malloc: worked
shown: segfaulted

hth

 

[David Farning]

I took a literary license and did some critiquing(sp?) of the code.

The direct answer to your question is marked.


declaring the struct in this manner will *not* make space on the stack
for the variable. Another way to do this (if you don't like doing
dynamic memory management for structs you know you'll need but don't want
to deal with the &'s all over the place) is:

struct stat _stbuf;
struct stat *stbuf = &_stbuf;

Either the program overflows the stack (and you should have a routine
registered to handle this event) or it works.

I also feel it's a bad idea to declare variables w/o initializing them
because a comparison to NULL later on might or might not reveal it as a
useless
pointer. Typically, NULL is 0, and things are *magically* zero out of
random luck, but that's a combination of implementation definitions and
luck.

---------------->ANSWER<--------------

The way you have it done as is, space is never being allocated for the
buffer. Consequently when the OS tries to find the file and fails, things
go as planned, but when it succeeds in finding the file, it tries to write
to a junk pointer, and thus segfaults.

i'm pretty sure // is illegal in ansi 89, but ok in 99

casting the return value of malloc is *bad*. Doing that will cause the
compiler not to complain that you forgot to include stdlib.




I'm not sure what editor you're using, but i'm pretty sure that industry
standard is two spaces, and a naming convention of lower case and _'s
instead of mixed case.

Thanks, I'm coming from a world mostly of java, python, and bash. Camel
case and tab(4) were pretty much the habit I got into. It is trully amazing
how many way there are to bite yourself in the butt with C--Not complaining
just grumping.

After removing the (struct stat*) cast it works like a charm. I thought
that it was important to cast pointers to the type of object that they
point to. Am I heading down a errorous path of thought.


Secondly, you mentioned are routine to handle stack overflows. Could you
point to more imformation on this.

Thanks
Dave Farning

 

[Gordon Burditt]

I am new to c and not yet sure of the boundary between c and it's

implementations. So please redirect me if this should be asked elsewhere.

I am working on a function to determine if a directory exists. The idea
comes from a snippet I found in a samba source file.

bool dirExist(char *dname)
{
struct stat *stbuf;

Here you have declared a pointer variable, but it isn't
initialized to point anywhere, so it might point at the Mars Rover
or it might point at your butt.

//stbuf = (struct stat*)malloc(sizeof(struct stat));
bool ret;

if (stat(dname,stbuf) != 0)

Here you are using an uninitialized pointer and passing it to
a function that expects to write data where the pointer is pointing.
Ouch! If that pointer is not pointing at your butt, YOU might be
the one who caused the problems with the Mars Rover.

Try declaring a struct stat and making stbuf point at it.

return(False);

ret = S_ISDIR(stbuf->st_mode);
if(!ret)
errno = ENOTDIR;
printf("%s\n",ret);
return ret;
}

It seems to agree with page 180 of K&R2.

When passed a none existant directory name, it correctly reports a failure.
Yet, when passed a existant dir it segfaults at

stat(dname,stbuf)

Where, exactly, do you think stbuf is pointing?
Why SHOULDN'T you get a smegmentation fault?

Gordon L. Burditt

 

[David Farning]

Here you have declared a pointer variable, but it isn't
initialized to point anywhere, so it might point at the Mars Rover
or it might point at your butt.


Here you are using an uninitialized pointer and passing it to
a function that expects to write data where the pointer is pointing.
Ouch! If that pointer is not pointing at your butt, YOU might be
the one who caused the problems with the Mars Rover.

I take offense to the mars rover comment--now that previous lander oops,
I'll admit too. That had nothing to do with a conversion error beween feet
and meters, rather it was due to a conversion error between pints and
liters at the local pub the night before I wrote the code to handle the
landing.

Dave Farning

 

[Jack Klein]

I took a literary license and did some critiquing(sp?) of the code.

The direct answer to your question is marked.


declaring the struct in this manner will *not* make space on the stack

^^^^^

What stack? No stack is defined or required by the C standard.

The point is that the OP created an uninitialized pointer. Accessing
the value of an uninitialized pointer produces undefined behavior, not
just attempting to dereference it.

for the variable. Another way to do this (if you don't like doing
dynamic memory management for structs you know you'll need but don't want to
deal with the &'s all over the place) is:

struct stat _stbuf;

While in this case there is no harm, it is generally a bad idea to
create identifiers beginning with a leading underscore. There are
specific cases where it is not allowed, and it is easier not to do so
at all than to remember the rules.

struct stat *stbuf = &_stbuf;

Either the program overflows the stack (and you should have a routine
registered to handle this event) or it works.

The stack has nothing to do with it. As far as the C standard is
concerned, the program can crash when the uninitialized pointer value
is accessed to pass to the function. The function might never
execute, let alone return a result.

I also feel it's a bad idea to declare variables w/o initializing them because
a comparison to NULL later on might or might not reveal it as a useless

There are good cases to be made both for initializing and not
initializing variables when they are defined. There would be no point
at all in initializing a pointer which would be assigned the value
returned by a call to malloc(), for example.

pointer. Typically, NULL is 0, and things are *magically* zero out of random
luck, but that's a combination of implementation definitions and luck.

Actually, NULL is required to be either 0 or (void *)0, or an
expression equivalent to one or the other. A null pointer may not be
represented by the pattern all bits 0, however.

---------------->ANSWER<--------------

The way you have it done as is, space is never being allocated for the buffer.
Consequently when the OS tries to find the file and fails, things go as
planned, but when it succeeds in finding the file, it tries to write to a junk
pointer, and thus segfaults.

i'm pretty sure // is illegal in ansi 89, but ok in 99

casting the return value of malloc is *bad*. Doing that will cause the
compiler not to complain that you forgot to include stdlib.
Agreed.




I'm not sure what editor you're using, but i'm pretty sure that industry
standard is two spaces, and a naming convention of lower case and _'s instead
of mixed case.

Neither two spaces nor any one particular naming convention is
"industry standard", for whatever unspecified industry you have in
mind.

 

[CBFalconer]

I am new to c and not yet sure of the boundary between c and it's
implementations. So please redirect me if this should be asked
elsewhere.

Google for N869 and download the text version. It is easily
searched in an editor or via grep (which you can't do with the
official version). If the function is not mentioned there, it is
non-standard. I think there is one exception, because N869 is
actually a draft.

 

[Joona I Palaste]

I take offense to the mars rover comment--now that previous lander oops,
I'll admit too. That had nothing to do with a conversion error beween feet
and meters, rather it was due to a conversion error between pints and
liters at the local pub the night before I wrote the code to handle the
landing.

None of this would have happened if the US had finally adopted the
metric system like *every single other bloody country in the world*.
They *said* they'd adopt it way back in the 19th century, and how much
have they done about it?

 

[Charles Harrison Caudill]

Charles Harrison Caudill wrote:

Secondly, you mentioned are routine to handle stack overflows. Could you
point to more imformation on this.

We're venturing out of the realm of the language, but the OS will typically
send your process a signal (this works at least on *nix's). You can handle
that signal intelligently.

google->c signal

hth

 

[Michael Wojcik]

Thanks, I'm coming from a world mostly of java, python, and bash. Camel
case and tab(4) were pretty much the habit I got into. It is trully amazing
how many way there are to bite yourself in the butt with C--Not complaining
just grumping.

*I* am pretty sure that there is no "industry standard" for either
indentation or mixed-case versus underscores in identifiers in C.
My advice would be to worry about the important stuff and ignore style
recommendations that don't have an obvious justification.

 

[Ben Pfaff]

*I* am pretty sure that there is no "industry standard" for either
indentation or mixed-case versus underscores in identifiers in C.

Definitely not. A company that I work for
WritesIdentifiersLikeThis but the GNU project
writes_them_like_this. It's just a style choice.