Segfault with c

[Hena]

This is annoying. I can't seem to figure out why this damn pro segfaults.

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

int main (void)
{ unsigned long result=0;
int i=0;
char test[]="123456,234567,345678,456789";

while (test[i]!='\0')
{ result=atol(&test[i]);
printf("%ld\n",result);
printf("j");
while (test[i]!=','||test[i]!='\0')
i++;
if (test[i]!='\0')
i++;
}
return 0;
}


Segmentation fault after printing first set of numbers. Before printing j.

[jemfinch]

Originally posted by Hena:
#include <stdio.h>
#include <stdlib.h>

int main (void)
{ unsigned long result=0;
int i=0;
char test[]="123456,234567,345678,456789";

while (test[i]!='\0')
{ result=atol(&test[i]);
printf("%ld\n",result);
printf("j");
while (test[i]!=','||test[i]!='\0')
i++;
if (test[i]!='\0')
i++;
}
return 0;
}


Segmentation fault after printing first set of numbers. Before printing j.[/QB]

First, atol takes a string, and keeps going until it sees a null character. So
if that was working, it's a quirk of your implementation and not
something you should rely on. atol is fully within its specification if it
chokes and does whatever when it runs up to a comma.

Second, that "j" is probably getting printed, but because of the line buffering
done on terminals, you're never seeing it.

Third, that should be an &&, not an || in your nested while loop.

This code is horrible broken in the first place due to the incorrect assumptions
about atol, though. If someone doesn't post the correct way to do it by the
time I have some free time, I'll do so.

Jeremy

[Hena]

I read from usage of atol (can't remeber the web page now), that it stops if it encounters non-digit on string. I quess it was not accurate info. Damn.

[Hena]

Well i got it going like it's needed to go.

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

int main (void)
{ unsigned long result=0;
int i=0,j=0;
char test[]="123456,234567,345678,456789",buffer[11];

while (test[i]!='\0')
{ for (j=0;test[i]!=','&&test[i]!='\0';j++,i++)
buffer[j]=test[i];
result=atol(buffer);
if (result==0)
continue;
printf("%ld\n",result);
if (test[i]!='\0');
i++;
}
return 0;
}



One thing that i can't understand though. Why there comes two 0 in the end, if i don't skip them with the extra if-statement?

[ 18 October 2001: Message edited by: Hena ]

[jemfinch]

I think the better way to do it would be use one simple while loop, here in
(pseudo?)code:


char s[] = "123,1234,12345";
int i = 0;
int accumulator = 0
while(s[i] != '\0') {
if(isdigit(s[i])) {
accumulator = 10*accumulator + (s[i]-'0')
} else {
printf("%ld\n", accumulator);
accumulator = 0;
}
++i;
}


I'm making no guarantees about that code, I don't write C often. But that's the
general idea.

Jeremy

[pinoy]

Originally posted by jemfinch:
First, atol takes a string, and keeps going until it sees a null character. So
if that was working, it's a quirk of your implementation and not
something you should rely on. atol is fully within its specification if it
chokes and does whatever when it runs up to a comma.

Second, that "j" is probably getting printed, but because of the line buffering
done on terminals, you're never seeing it.

Third, that should be an &&, not an || in your nested while loop.

This code is horrible broken in the first place due to the incorrect assumptions
about atol, though. If someone doesn't post the correct way to do it by the
time I have some free time, I'll do so.

Jeremy[/QB]

First, that is a string he/she's passing.
&test[i], gives the substring starting at i. It is not broken because of the assumption of atol.

The bug is the '||' instead of '&&'. It's a buffer overrun.

[pinoy]

Oh and here's the first method:

int main(void)
{
char *test = "1234,2345,3456,4567";
char *p = test;
char *start = p;
while (*start) {
if (isdigit(*p)) p++;
else {
printf("%d\n", atol(start));
start = ++p;
}
}

return 0;
}


And the accumulator version that works. (your version doesn't print out the last number)


int main(void)
{
char *test = "1234,2345,3456,4567";
char *p = test;
int accumulator = 0;
do {
if (isdigit(*p))
accumulator = accumulator * 10 + (*p - '0');
else
printf("%d\n", accumulator), accumulator = 0;
} while (*p++);
return 0;
}

[Hena]

Posted by Pinoy:
The bug is the '||' instead of '&&'. It's a buffer overrun.


Could you tell me then why it doesn't print that 'f' as it is before the bug? Thanks to all for helping out btw.

[kmj]

When you do output in C, it buffers it, meaning it saves it in memory until some point; then it flushes it, meaning it finally prints out all the output. The reason for this is efficiency, output is inefficient, so it's better to make one long output than a bunch of short ones. Thing is, when you buffer, if your program exits unexpectedly, it may never get to output that data. Generally, most systems flush on a newline (\n); there's probably some way to get it to flush on other characters, too, or just some function to flush the output, but I don't know, I've never needed to deal with it.

[bwkaz]

If you absolutely have to flush your buffer, look at fflush(3).

Basically, you call fflush(stdout);

[sans-hubris]

I'm going to comment your code:
#include <stdio.h>
#include <stdlib.h>

int main (void)
{ unsigned long result=0;
int i=0;
char test[]="123456,234567,345678,456789";

while (test[i]!='\0')
{ result=atol(&test[i]);
printf("%ld\n",result);
printf("j");
while (test[i]!=','||test[i]!='\0')// What happens when you reach '\0'? What is (test[i]!=',')?
i++;
if (test[i]!='\0')
i++;
}
return 0;
}


Segmentation fault after printing first set of numbers. Before printing j.[/qb]<HR></BLOCKQUOTE>

[ 19 October 2001: Message edited by: Muad Dib --formerly ndogg ]