Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > segfault w/ block, but not file scope

Reply
Thread Tools

segfault w/ block, but not file scope

 
 
M.B
Guest
Posts: n/a
 
      01-06-2006
This works without segfault.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <dirent.h>
#include <errno.h>

#define H5DIR "./"

int getFileNames(char *directory, struct dirent ***namelist);
int freeFileNames(struct dirent **namelist, int entries);


int main(void)
{
int n,err;
struct dirent **namelist;

n = getFileNames(H5DIR,&namelist);
printf("%d\n",n);
err = freeFileNames(namelist, n);
if (err==0)
printf("There wasn't any files");
return 0;

}

int getFileNames(char *directory, struct dirent ***namelist)
{
int n, i;

n = scandir(directory, namelist, 0, alphasort);
if(n == -1){
perror("scandir returned: ");
exit(1);
}
if(n == 0){
printf("No files found. Quiting.\n\n");
exit(1);
}
for (i=0;i<n;i++){
printf("File: %s\n", (*namelist)[i]->d_name);
}

return n;

}

int freeFileNames(struct dirent **namelist, int entries)
{
int i;

if (namelist == NULL)
return 0;
else{
printf("%d",entries);
for (i = 0; i < entries; i++){
free(namelist[i]);
namelist[i] = NULL;
printf("%d,", i);
}
puts("\n");
free(namelist);
namelist = NULL;
}

return 1;
}

 
Reply With Quote
 
 
 
 
Dieter
Guest
Posts: n/a
 
      01-06-2006
Chuck F. wrote:
> Dieter wrote:
>
>> Jack Klein wrote:
>>
>>> [snip]
>>>
>>> On the last line of code above, you pass a pointer to
>>> 'namelist' (therefore a char ***) to scandir(). Presumably
>>> this function checks whether the pointed-to char ** is NULL or
>>> not, and if it is NULL, it allocates the necessary memory.
>>> And also presumably, if the pointed-to char ** is not NULL, it
>>> assumes that it points to valid memory and uses it. At the
>>> end you try to free a pointer that was not allocated.
>>>
>>> I would suggest that you study the documentation for the
>>> function scandir() and see what the requirements are for that
>>> parameter.

>>
>>
>> Jack, I sincerely thank you for commenting.
>>
>> NULL initialization was certainly an issue.

>
>
> All of which points out why we want queries to be topical. Once you
> involve an unknown header (dirent) and an unknown function (scandir)
> nobody can have any accurate idea what goes on. This is why this whole
> thread should have been on a newsgroup dealing with your system in the
> first place.
>
> The fact that Jack could make an educated guess does not affect this.
> His guess could well have been total nonsense, and could have missed
> important factors. There is (in principle) noone here to correct any
> mistakes.
>


Yes I appreciate that.

My question, I believed, was standard C specific and debated whether to
post it here. In the future *anything* including anything other than
standard C will be posted elsewhere.

Possibly the newsgroup's name could be changed to comp.lang.C.standard,
or some such. It might eliminate a small amount of the regular grief you
all encounter.

Regards,
Dieter
 
Reply With Quote
 
 
 
 
M.B
Guest
Posts: n/a
 
      01-06-2006

Chuck F. wrote:
> Dieter wrote:
> > Jack Klein wrote:
> >
> >> [snip]
> >>
> >> On the last line of code above, you pass a pointer to
> >> 'namelist' (therefore a char ***) to scandir(). Presumably
> >> this function checks whether the pointed-to char ** is NULL or
> >> not, and if it is NULL, it allocates the necessary memory.
> >> And also presumably, if the pointed-to char ** is not NULL, it
> >> assumes that it points to valid memory and uses it. At the
> >> end you try to free a pointer that was not allocated.
> >>



> >> I would suggest that you study the documentation for the
> >> function scandir() and see what the requirements are for that
> >> parameter.

> >
> > Jack, I sincerely thank you for commenting.
> >
> > NULL initialization was certainly an issue.

>
> All of which points out why we want queries to be topical. Once
> you involve an unknown header (dirent) and an unknown function
> (scandir) nobody can have any accurate idea what goes on. This is
> why this whole thread should have been on a newsgroup dealing with
> your system in the first place.
>

btw dirent or dirent.h is not alien. its of course not in ansi but in
posix (k&r also mentions it)

> The fact that Jack could make an educated guess does not affect
> this. His guess could well have been total nonsense, and could
> have missed important factors. There is (in principle) noone here
> to correct any mistakes.
>


I agree here. Anyone replying here want to help in any case. But we all
can help in giving suggestions but not spoonfeed. I may not know what
others know . knowlegde sharing is the idea here.

> --
> "If you want to post a followup via groups.google.com, don't use
> the broken "Reply" link at the bottom of the article. Click on
> "show options" at the top of the article, then click on the
> "Reply" at the bottom of the article headers." - Keith Thompson
> More details at: <http://cfaj.freeshell.org/google/>


 
Reply With Quote
 
Dieter
Guest
Posts: n/a
 
      01-06-2006
M.B wrote:
> please check
> pass by value
> v/s pass by ref/pointers carefully
>
>
> hope this helps to solve the issue
>


Thank you for your comments MB.
 
Reply With Quote
 
M.B
Guest
Posts: n/a
 
      01-06-2006

Chuck F. wrote:
> Dieter wrote:
> > Jack Klein wrote:
> >
> >> [snip]
> >>
> >> On the last line of code above, you pass a pointer to
> >> 'namelist' (therefore a char ***) to scandir(). Presumably
> >> this function checks whether the pointed-to char ** is NULL or
> >> not, and if it is NULL, it allocates the necessary memory.
> >> And also presumably, if the pointed-to char ** is not NULL, it
> >> assumes that it points to valid memory and uses it. At the
> >> end you try to free a pointer that was not allocated.
> >>



> >> I would suggest that you study the documentation for the
> >> function scandir() and see what the requirements are for that
> >> parameter.

> >
> > Jack, I sincerely thank you for commenting.
> >
> > NULL initialization was certainly an issue.

>
> All of which points out why we want queries to be topical. Once
> you involve an unknown header (dirent) and an unknown function
> (scandir) nobody can have any accurate idea what goes on. This is
> why this whole thread should have been on a newsgroup dealing with
> your system in the first place.
>

btw dirent or dirent.h is not alien. its of course not in ansi but in
posix (k&r also mentions it)

> The fact that Jack could make an educated guess does not affect
> this. His guess could well have been total nonsense, and could
> have missed important factors. There is (in principle) noone here
> to correct any mistakes.
>


I agree here. Anyone replying here want to help in any case. But we all
can help in giving suggestions but not spoonfeed. I may not know what
others know . knowlegde sharing is the idea here.

> --
> "If you want to post a followup via groups.google.com, don't use
> the broken "Reply" link at the bottom of the article. Click on
> "show options" at the top of the article, then click on the
> "Reply" at the bottom of the article headers." - Keith Thompson
> More details at: <http://cfaj.freeshell.org/google/>


 
Reply With Quote
 
Dieter
Guest
Posts: n/a
 
      01-06-2006
Dieter wrote:
> Hi.
>
> In the snippet of code below, I'm trying to understand why when the
>
> struct dirent ** namelist
>
> is declared with "file" scope, I don't have a problem freeing the
> allocated memory. But when the struct is declared in main (block scope)
> it will segfault when passing namelist to freeFileNames().
>
> Since this seems to be just a matter of my understanding scope and
> pointer parameter passing better, I only included what thought to be
> relevant code. I'll happily provide compilable code if deemed necessary.
>
> Please see commented lines:
>
>
> struct dirent **namelist; /* file scope works */
>
> int main(void)
> {
> /* struct dirent **namelist */ /* block scope doesn't work */
> int n;
>
> n = getFileNames(H5DIR, namelist); /* included from mylib.h */
> freeFileNames(namelist, n); /* included from mylib.h */
>
> return 0;
> }
>
>
> Thank you very much for your comments,
> Dieter


My apologies to the regulars for an OT inclusion related to the question.


< This Thread Closed >


Thanks,
Dieter
 
Reply With Quote
 
Dieter
Guest
Posts: n/a
 
      01-06-2006
Chris Torek wrote:
> In article <(E-Mail Removed)>
> Dieter <(E-Mail Removed)> wrote:
>
>>struct dirent **namelist; /* file scope works */
>>
>>int main(void)
>>{
>> /* struct dirent **namelist */ /* block scope doesn't work */
>> int n;
>>
>> n = getFileNames(H5DIR, namelist); /* included from mylib.h */

>
>
> This passes the *value* of the variable "namelist" to the function.
> If namelist has block scope, the variable is also automatic, and hence
> uninitialized, and hence contains garbage. If namelist has file
> scope, the variable also has static duration and hence is
> initialized to NULL.
>
> In any case, getFileNames() cannot change the value stored in
> the variable named "namelist" (unless it does not declare one
> of its own, and does access the file-scope one). So it makes
> no sense to pass the value in the first place, if namelist is
> a block-scope variable.
>
> If getFileNames() needs to modify namelist, it will need the
> address of the variable:
>
> extern int getFileNames(first_type, struct dirent ***);
> ...
> int main(void) {
> struct dirent **namelist;
> int n;
>
> /* optional: namelist = NULL; */
> n = getFileNames(H5DIR, &namelist);
>
>
>> freeFileNames(namelist, n); /* included from mylib.h */

>
>
> Presumably the NULL-initialized file-scope static-duration version
> causes freeFileNames() to be a no-op. Passing the uninitialized
> value of the block-scope version presumably does not.
>
>
>> return 0;
>>}


You've hit the nail on the head. That's exactly what I've been having
trouble wrapping my mind around.

* Pass the address of the pointer in order to return it's value modified. *

The N indirection, dynamic allocation, and passing them has been
confusing me.


Thanks Chris
 
Reply With Quote
 
Dieter
Guest
Posts: n/a
 
      01-06-2006
M.B wrote:
> This works without segfault.
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <dirent.h>
> #include <errno.h>
>
> #define H5DIR "./"
>
> int getFileNames(char *directory, struct dirent ***namelist);
> int freeFileNames(struct dirent **namelist, int entries);
>
>
> int main(void)
> {
> int n,err;
> struct dirent **namelist;
>
> n = getFileNames(H5DIR,&namelist);
> printf("%d\n",n);
> err = freeFileNames(namelist, n);
> if (err==0)
> printf("There wasn't any files");
> return 0;
>
> }
>
> int getFileNames(char *directory, struct dirent ***namelist)
> {
> int n, i;
>
> n = scandir(directory, namelist, 0, alphasort);
> if(n == -1){
> perror("scandir returned: ");
> exit(1);
> }
> if(n == 0){
> printf("No files found. Quiting.\n\n");
> exit(1);
> }
> for (i=0;i<n;i++){
> printf("File: %s\n", (*namelist)[i]->d_name);
> }
>
> return n;
>
> }
>
> int freeFileNames(struct dirent **namelist, int entries)
> {
> int i;
>
> if (namelist == NULL)
> return 0;
> else{
> printf("%d",entries);
> for (i = 0; i < entries; i++){
> free(namelist[i]);
> namelist[i] = NULL;
> printf("%d,", i);
> }
> puts("\n");
> free(namelist);
> namelist = NULL;
> }
>
> return 1;
> }
>

MB, I can't express enough how much I appreciate your contribution.
Thank you for taking the time... and bending the sacred rules a little.
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      01-06-2006
"Chuck F. " <(E-Mail Removed)> writes:
> Dieter wrote:
>> Jack Klein wrote:
>>> [snip]
>>> On the last line of code above, you pass a pointer to
>>> 'namelist' (therefore a char ***) to scandir(). Presumably
>>> this function checks whether the pointed-to char ** is NULL or
>>> not, and if it is NULL, it allocates the necessary memory.
>>> And also presumably, if the pointed-to char ** is not NULL, it
>>> assumes that it points to valid memory and uses it. At the
>>> end you try to free a pointer that was not allocated.
>>> I would suggest that you study the documentation for the
>>> function scandir() and see what the requirements are for that
>>> parameter.

>> Jack, I sincerely thank you for commenting.
>> NULL initialization was certainly an issue.

>
> All of which points out why we want queries to be topical. Once you
> involve an unknown header (dirent) and an unknown function (scandir)
> nobody can have any accurate idea what goes on. This is why this
> whole thread should have been on a newsgroup dealing with your system
> in the first place.
>
> The fact that Jack could make an educated guess does not affect this.
> His guess could well have been total nonsense, and could have missed
> important factors. There is (in principle) noone here to correct any
> mistakes.


In this case, there wasn't really any need to guess. The code causing
the problem was:

struct dirent **namelist;
int n;
n = getFileNames(H5DIR, namelist);

It might as well have been:

struct foo **bar;
int n;
n = bleeble(blah, bar);

Regardless of what struct dirent/struct foo is, and regardless of what
getFileNames() or bleeble() does with its arguments, passing an
uninitialized variable to a function is almost certainly an error.
(Come to think of it, drop the "almost".)

--
Keith Thompson (The_Other_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <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.
 
Reply With Quote
 
tmp123
Guest
Posts: n/a
 
      01-06-2006
Dieter wrote:
> int getFileNames(char *directory, struct dirent **namelist)
> {
> int n, i;
>
> n = scandir(directory, &namelist, 0, alphasort);



Hi,

As other groups members has remarked, the problem was in the call to
scandir (the program doesn't works if the variable is global and also
doesn't works if it is local. The only diference is how bad are the
effects).

I want only to remark two things about modify a function parameter:
a) The changes done to the value of a parameter will be always lost at
function return.
b) Change a parameter is under stylistic discussion (in particular, I
never do it. If necessary, I declare a local and init it with the
parameter value), and, who knows if allowed by standard. By example, a
SPARC CPU uses registers to pass parameters, so, the address of a
parameter is something that breaks to normal function prologs
(assembler code at start of fucntion).

In conclusion, any line of the kind "&parameter" must be carefully
reviewed.

Kind regards.

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
invoking a segfault within a segfault handler - is this undefinedbehavior? Andrey Vul C Programming 8 07-30-2010 02:14 PM
GCC gives SEGFAULT... but GDB runs sethukr@gmail.com C Programming 6 03-02-2007 05:53 PM
Scope - do I need two identical classes, each with different scope? ann Java 13 09-13-2005 03:07 AM
How do namespace scope and class scope differ? Steven T. Hatton C++ 9 07-19-2005 06:07 PM
IMPORT STATIC; Why is "import static" file scope? Why not class scope? Paul Opal Java 12 10-10-2004 11:01 PM



Advertisments