having problems with pointers (segmentation fault)

If I run the following I get a segmentation fault:

#define NAMELEN 15
#define NPERS 10

typedef struct pers {
char name[NAMELEN+1];
int money;
} pers_t;

char *my_fgets(char *s, int n, FILE *f) {
size_t i;
fgets(s, n+2, f);
i = strlen(s)-1;
if(s[(int)i] == '\n')
s[(int)i] = '\0';
return s;
}


int main(int argc, char *argv[]) {
int numOfPers;
pers_t pers[NPERS];
char s[NAMELEN+1];
FILE *in = fopen("gift1.in", "r");

/* initialize names for comparison */
for(i = 0; i < NPERS; i++)
bzero(pers[i].name, NAMELEN);

numOfPers = atoi(my_fgets(s, NAMELEN, in));
#ifdef DEBUG
printf("number of persons: %d\n", numOfPers);
my_fgets(pers[0].name, NAMELEN, in);
printf("person number 1 is %s\n", pers[0].name);
my_fgets(pers[1].name, NAMELEN, in);
printf("person number 2 is %s\n", pers[1].name);
#endif

for(i = 0; i < numOfPers; i++) {
my_fgets(pers[i].name, NAMELEN, in);
#ifdef DEBUG
printf("person number %d is %s", i+1, pers[i].name);
#endif
}

What seems so strange to me is that when I read the names in
'manually' it works perfectely, but when I try to read them in the for
loop the program crashes. I know that the error occurs in the for loop
because I won't see the result of the printf in the for loop. The
output is as follows:

number of persons: 5
person number 1 is dave
person number 2 is laura
Segmentation fault

Has anybody an idea of why this is?
Thanks
damian

re: having problems with pointers (segmentation fault)

damian birchler wrote:[color=blue]
> If I run the following I get a segmentation fault:
>[/color]
You need two headers.

#include <stdio.h> /* FILE */
#include <stdlib.h> /* EXIT_SUCCESS */
#include <string.h> /* strlen */
[color=blue]
> #define NAMELEN 15
> #define NPERS 10
>
> typedef struct pers {
> char name[NAMELEN+1];
> int money;
> } pers_t;
>
> char *my_fgets(char *s, int n, FILE *f) {
> size_t i;
> fgets(s, n+2, f);
> i = strlen(s)-1;
> if(s[(int)i] == '\n')[/color]

Why are you casting it to 'int' ?
It is good to have the array indices to be of type 'size_t' .

[color=blue]
> s[(int)i] = '\0';[/color]

[color=blue]
> return s;
> }
>
>
> int main(int argc, char *argv[]) {
> int numOfPers;
> pers_t pers[NPERS];
> char s[NAMELEN+1];
> FILE *in = fopen("gift1.in", "r");[/color]

What if file open fails ?
[color=blue]
>
> /* initialize names for comparison */
> for(i = 0; i < NPERS; i++)
> bzero(pers[i].name, NAMELEN);
>[/color]
Please use memset with 0 instead of bzero .
bzero is non-standard.

memset(pers[i].name, 0, NAMELEN);
[color=blue]
> numOfPers = atoi(my_fgets(s, NAMELEN, in));
> #ifdef DEBUG
> printf("number of persons: %d\n", numOfPers);
> my_fgets(pers[0].name, NAMELEN, in);
> printf("person number 1 is %s\n", pers[0].name);
> my_fgets(pers[1].name, NAMELEN, in);
> printf("person number 2 is %s\n", pers[1].name);
> #endif[/color]

In DEBUG mode, you have already read 2 persons' name.
[color=blue]
>
> for(i = 0; i < numOfPers; i++) {[/color]

#ifdef DEBUG
for(i = 0; i < numOfPers - 2; i++) {
#else
for(i = 0; i < numOfPers; i++) {
#endif

assuming you have read 2 persons' name before the loop.

[color=blue]
> my_fgets(pers[i].name, NAMELEN, in);
> #ifdef DEBUG
> printf("person number %d is %s", i+1, pers[i].name);
> #endif
> }
>
> What seems so strange to me is that when I read the names in
> 'manually' it works perfectely, but when I try to read them in the for
> loop the program crashes. I know that the error occurs in the for loop
> because I won't see the result of the printf in the for loop. The
> output is as follows:
>
> number of persons: 5
> person number 1 is dave
> person number 2 is laura
> Segmentation fault[/color]

My gift1.in looks as follows -

3
ADAM
BEN
CHRIS
DENNIS
ERIC


It worked for me.

re: having problems with pointers (segmentation fault)

"damian birchler" <damian_birchler@bluewin.ch> wrote[color=blue]
> If I run the following I get a segmentation fault:
>
> #define NAMELEN 15
> #define NPERS 10
>
> typedef struct pers {
> char name[NAMELEN+1];
>[/color]
You can have a name of maximum NAMELEN characters.[color=blue]
>
> int money;
> } pers_t;
>
> char *my_fgets(char *s, int n, FILE *f) {
> size_t i;
> fgets(s, n+2, f);
>[/color]
This is asking for trouble. First, you should check the return value.
Secondly, the length parameter gives the buffer size. By passing n+2, you
risk a memory overun of one.[color=blue]
>
> i = strlen(s)-1;
> if(s[(int)i] == '\n')
> s[(int)i] = '\0';
>[/color]
The cast is unnecessary, and in fact disguises a problem. What if the call
to fgets() returns an empty string? You will attempt to read from s[-1],
which could be another segmentation fault.[color=blue]
>
> return s;
> }
>
>
> int main(int argc, char *argv[]) {
> int numOfPers;
> pers_t pers[NPERS];
> char s[NAMELEN+1];
> FILE *in = fopen("gift1.in", "r");
>
> /* initialize names for comparison */
> for(i = 0; i < NPERS; i++)
> bzero(pers[i].name, NAMELEN);
>
> numOfPers = atoi(my_fgets(s, NAMELEN, in));
> #ifdef DEBUG
> printf("number of persons: %d\n", numOfPers);
> my_fgets(pers[0].name, NAMELEN, in);
> printf("person number 1 is %s\n", pers[0].name);
> my_fgets(pers[1].name, NAMELEN, in);
> printf("person number 2 is %s\n", pers[1].name);
> #endif
>
> for(i = 0; i < numOfPers; i++) {
> my_fgets(pers[i].name, NAMELEN, in);
> #ifdef DEBUG
> printf("person number %d is %s", i+1, pers[i].name);
> #endif
> }
>
> What seems so strange to me is that when I read the names in
> 'manually' it works perfectely, but when I try to read them in the for
> loop the program crashes. I know that the error occurs in the for loop
> because I won't see the result of the printf in the for loop. The
> output is as follows:
>
> number of persons: 5
> person number 1 is dave
> person number 2 is laura
> Segmentation fault
>
> Has anybody an idea of why this
>[/color]
A segmentation fault is caused by an undefined-behaviour read or write to
memory you do not own. Since the behaviour is undefined, seemingly unrelated
changes such as unrolling a for loop may cause the error to crop up in a
different place. This could be because you changed the memory layout of the
program internally.

re: having problems with pointers (segmentation fault)

> If I run the following I get a segmentation fault:[color=blue]
>
> #define NAMELEN 15
> #define NPERS 10
>
> typedef struct pers {
> char name[NAMELEN+1];
> int money;
> } pers_t;
>
> char *my_fgets(char *s, int n, FILE *f) {[/color]

Change the type of ``n'' to ``size_t''.
If you have an fgets wrapper that relies on the size
of an object, then you want to be consistent with
the fgets function, which accepts the size of an object
as a size_t.
[color=blue]
> size_t i;
> fgets(s, n+2, f);[/color]

And you pass NAMELEN to my_fgets, then you add
two here? Why? Do you realize what you are doing?
The size of the object is NAMELEN+1, so what is
this additional two for? You are lying to fgets about the
size. fgets can now read in one less than ``n+2'' bytes
starting from 0, so that is 17 bytes while ``name'' is 16 bytes.
[color=blue]
> i = strlen(s)-1;
> if(s[(int)i] == '\n')[/color]

I want you to reply giving reasoning for this cast.
[color=blue]
> s[(int)i] = '\0';[/color]

Likewise.
[color=blue]
> return s;
> }
>
>
> int main(int argc, char *argv[]) {
> int numOfPers;
> pers_t pers[NPERS];
> char s[NAMELEN+1];
> FILE *in = fopen("gift1.in", "r");
>[/color]

You'll want to ensure that fopen did not fail,
so check if ``in'' is equal to NULL.
[color=blue]
> /* initialize names for comparison */
> for(i = 0; i < NPERS; i++)[/color]

I don't see where ``i'' is declared.
[color=blue]
> bzero(pers[i].name, NAMELEN);[/color]

In this newsgroup, we discuss programming
under the c89/90 and c99 abstract machine,
which ``bzero'' is not a part of.

[color=blue]
>
> numOfPers = atoi(my_fgets(s, NAMELEN, in));[/color]

What do you think happens when ``my_fgets''
returns a string which represents an integer
value larger than what an int can portably represent?

Use strtol for error checking while changing the
type of ``numOfPers'' to long. When you do so,
you will also need to change the conversion
``%d'' to include the ``l''(That is, ELL) modifier.
So ``%ld''.
[color=blue]
> #ifdef DEBUG
> printf("number of persons: %d\n", numOfPers);
> my_fgets(pers[0].name, NAMELEN, in);
> printf("person number 1 is %s\n", pers[0].name);
> my_fgets(pers[1].name, NAMELEN, in);
> printf("person number 2 is %s\n", pers[1].name);
> #endif
>
> for(i = 0; i < numOfPers; i++) {
> my_fgets(pers[i].name, NAMELEN, in);
> #ifdef DEBUG
> printf("person number %d is %s", i+1, pers[i].name);
> #endif
> }
>[/color]

If DEBUG is defined, what do you think will happen
when your variadic function is called without a prototype
in scope?



--
j

re: having problems with pointers (segmentation fault)

damian birchler wrote:[color=blue]
> If I run the following I get a segmentation fault:
>
> #define NAMELEN 15
> #define NPERS 10
>
> typedef struct pers {
> char name[NAMELEN+1];
> int money;
> } pers_t;
>
> char *my_fgets(char *s, int n, FILE *f) {
> size_t i;
> fgets(s, n+2, f);[/color]
^^^
This is nuts. Since the size of the name member is NAMELEN+1, and
n==NAMELEN, it is insane to purposely ask for a buffer overrun.
[color=blue]
> pers_t pers[NPERS];[/color]
[color=blue]
> my_fgets(pers[0].name, NAMELEN, in);[/color]

Do yourself a favor, and make NAMELEN reflect the '\n\0' and stop
screwing around with inconsistent additions.

re: having problems with pointers (segmentation fault)

Ok. It was my fault. I wasn't exact enough. I'll try to fix that all
at once. This time I will post the whole program. The program is an
aproach to a problem at the IOI training program, so I'm not concerned
about malformed input (should I ?), nor am I concerned about faling
fopens (I wouldn't know how to react).

/*
ID: damian_1
PROG: gift1
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#define NAMELEN 15 /* extra space for newline */
#define NPERS 10

typedef struct pers {
char name[NAMELEN+1];
int money;
} pers_t;

char *my_fgets(char *s, int n, FILE *f) { /* like fgets but without
newline */
fgets(s, (size_t)(n+1), f);
s[strlen(s)-1] = '\0';
return s;
}

int findPers(char *name, int numOfPers, pers_t *pers) {
int i;
for(i = 0; i < numOfPers; i++)
if(strcmp(name, pers[i].name) == 0)
return i;
}

int main(int argc, char *argv[]) {
int numOfPers;
int numOfGifts;
int initMoney;
int remainMoney;
int amount;
pers_t pers[NPERS];
int i;
int j;
char **ptr;
char s[NAMELEN+1];
FILE *in = fopen("gift1.in", "r");
FILE *out = fopen("gift1.out", "w");

/* initialize money */
for(i = 0; i < NPERS; i++)
pers[i].money = 0;

numOfPers = atoi(my_fgets(s, NAMELEN, in));

/* read in the persons' names */
for(i = 0; i < numOfPers; i++)
my_fgets(pers[i].name, NAMELEN, in);

/* for each person get its initial money, allot it and update it */
for(i = 0; i < numOfPers; i++) {
j = findPers(my_fgets(s, NAMELEN, in), numOfPers, pers);
initMoney = (int)strtol(my_fgets(s, NAMELEN, in), ptr, 0);
numOfGifts = (int)strtol(*ptr, NULL, 0);
remainMoney = initMoney % numOfGifts;
amount = (initMoney - remainMoney) / numOfGifts;
pers[j].money -= initMoney + remainMoney;
for(j = 0; j < numOfGifts; j++)
pers[findPers(my_fgets(s, NAMELEN, in), numOfPers,
pers)].money += amount;
}

/* print out solution */
for(i = 0; i < numOfPers; i++)
fprintf(out, "%s %d\n", pers[i].name, pers[i].money);

return 0;
}

re: having problems with pointers (segmentation fault)

On 8 Oct 2004 02:44:24 -0700, damian_birchler@bluewin.ch (damian
birchler) wrote:
[color=blue]
>Ok. It was my fault. I wasn't exact enough. I'll try to fix that all
>at once. This time I will post the whole program. The program is an
>aproach to a problem at the IOI training program, so I'm not concerned
>about malformed input (should I ?), nor am I concerned about faling
>fopens (I wouldn't know how to react).
>
>/*
>ID: damian_1
>PROG: gift1
>*/
>
>#include <stdio.h>
>#include <stdlib.h>
>#include <string.h>
>#include <ctype.h>
>
>#define NAMELEN 15 /* extra space for newline */
>#define NPERS 10
>
>typedef struct pers {
> char name[NAMELEN+1];
> int money;
>} pers_t;
>
>char *my_fgets(char *s, int n, FILE *f) { /* like fgets but without
>newline */
> fgets(s, (size_t)(n+1), f);[/color]

The cast is unnecessary since the prototype is in scope.
[color=blue]
> s[strlen(s)-1] = '\0';[/color]

You have not checked to insure that the character about to be replaced
is in fact a '\n'. You could be replacing a valid character in the
name.
[color=blue]
> return s;
>}
>
>int findPers(char *name, int numOfPers, pers_t *pers) {
> int i;
> for(i = 0; i < numOfPers; i++)
> if(strcmp(name, pers[i].name) == 0)
> return i;[/color]

What happens if the name is not found. What do you return?
[color=blue]
>}
>
>int main(int argc, char *argv[]) {
> int numOfPers;
> int numOfGifts;
> int initMoney;
> int remainMoney;
> int amount;
> pers_t pers[NPERS];
> int i;
> int j;
> char **ptr;
> char s[NAMELEN+1];
> FILE *in = fopen("gift1.in", "r");[/color]

You never check if fopen succeeds.
[color=blue]
> FILE *out = fopen("gift1.out", "w");[/color]

Ditto.
[color=blue]
>
> /* initialize money */
> for(i = 0; i < NPERS; i++)
> pers[i].money = 0;
>
> numOfPers = atoi(my_fgets(s, NAMELEN, in));[/color]

If the fopen failed, all the calls to my_fgets invoke undefined
behavior which can manifest as a seg fault.
[color=blue]
>
> /* read in the persons' names */
> for(i = 0; i < numOfPers; i++)
> my_fgets(pers[i].name, NAMELEN, in);
>
> /* for each person get its initial money, allot it and update it */
> for(i = 0; i < numOfPers; i++) {
> j = findPers(my_fgets(s, NAMELEN, in), numOfPers, pers);
> initMoney = (int)strtol(my_fgets(s, NAMELEN, in), ptr, 0);
> numOfGifts = (int)strtol(*ptr, NULL, 0);
> remainMoney = initMoney % numOfGifts;
> amount = (initMoney - remainMoney) / numOfGifts;
> pers[j].money -= initMoney + remainMoney;[/color]

If findPers failed, this would also invoke undefined behavior.
[color=blue]
> for(j = 0; j < numOfGifts; j++)
> pers[findPers(my_fgets(s, NAMELEN, in), numOfPers,
>pers)].money += amount;
> }
>
> /* print out solution */
> for(i = 0; i < numOfPers; i++)
> fprintf(out, "%s %d\n", pers[i].name, pers[i].money);[/color]

If the fopen failed, this also would invoke undefined behavior.
[color=blue]
>
> return 0;
>}[/color]



<<Remove the del for email>>