Re: Problem Debugging C Program



Ben Bacarisse wrote:
John Hanley <hanley@xxxxxxxxxxx> writes:

Ben Pfaff wrote:
John Hanley <hanley@xxxxxxxxxxx> writes:

Sounds good. In the meantime, here is the code if you have the time to
take a peek at it.
Here's one problem:

blp@blp:~(1)$ gcc -Wall foo.c
foo.c: In function 'extract_records':
foo.c:472: warning: unused variable 'max_fields'
foo.c: In function 'main':
foo.c:613: warning: format '%s' expects type 'char *', but argument 3 has type 'struct FILE *'

I cleared both of these out and still no dice.

First, the good news. I strongly suspect you are hitting this problem:

case 4: strcpy(et,field); /* get elapsed time as string */
break;

et has not been given anywhere to point. It is an uninitialised 'char
*'.


I think that did it. I REALLLLY appreciate you taking this much time to look at my code and provide pointers. You gave some excellent tips. I am a young programmer and tips like this help me to learn and get better and better.

This is great stuff and I thank you, and everyone who provided tips, very, very, much!

Sincerely,
John



The bad news: you need to fix a lot of other stuff! I found it very
hard to coax you code to take any data without hitting buffer
overflows and so on. You need to find a more robust way to parse the
data.

Take, for example this:

char f[256]={0};
int i=0;
char c='\0';
int po;

po=*ptr_offset;

This is confusing to old-timers. =* used to be how we now write *=.

while(1)
{
sscanf(line+po,"%c",&c); /* read in a char from line */

This is a loooong way to write 'c = line[po];'

if((c==DELIMITER) || (c=='\n') || (c==EOF))

c is unlikely to be == EOF because EOF is a negative int and c is a
char. You probably mean to test for the end of the string here (c ==
'\0') rather than for EOF which either happened long ago when the line
was read or will happen some time in the future.

Also, DELIMITER suggests a macro name and it might be better to use
one here (you have DELIMITER as a char variable set to '\t').

break;
f[i]=c;
i++;
po++;
}

f[i]='\0'; /* null terminate string */

All this to put the first tab delimited field into f. It is usully
better to make ones loop explicit and up-font:

while ((c = line[po]) != '\t' && c != '\n' && c != '\0') {
if (i < sizeof f - 1)
f[i++] = c;
po++;
}
f[i] = '\0';

I probably would not write it like this, but I want to show an
incremental change you can make. Note that I check there is room for
the data in f;

Later, there is this:

char ampm[3];
...
sscanf(field+po, "%s",ampm);

if( (strcmp(ampm,"pm")==0) || (strcmp(ampm,"PM")==0) )
add=12;

This is dangerous (you get a buffer overrun in the input does not have
exactly a two-character string at this point) but also long winded.
You don't use 'ampm' later, so you can just test:

if ((strcmp(line+po, "pm") == 0) || strcmp(line+po, "PM") == 0))
add=12;

It would help if got familiar with what C provides for doing string
manipulation. One function you will want to learn is strtol -- it
converts to an integer (like the atoi you use) put it reports on error
and how much it "consumed" so you don't need to do stuff like this:

sscanf(field,"%s",date_str);
po=po+strlen(date_str)+1;

sscanf(field+po, "%s",time_str);
po=po+strlen(time_str)+1;

.



Relevant Pages

  • Re: socket communication: send & receive doesnt work right
    ... So I don't want to send a string as bytes. ... public void send_doubles(double vals, int len) throws ... // send a short acknowledgement to the server ... char *result; ...
    (microsoft.public.win32.programmer.networks)
  • Re: [PATCH] markers: modpost
    ... pointers to the name/format string pairs. ... The same can then be done with modules using the __markers section. ... +static void read_markers(const char *fname) ... int main ...
    (Linux-Kernel)
  • Re: [PATCH] markers: modpost
    ... This adds some new magic in the MODPOST phase for CONFIG_MARKERS. ... will be a neighbor of its format string. ... +static void read_markers(const char *fname) ... int main ...
    (Linux-Kernel)
  • Re: Is this code totaly a shit?
    ... | void UppStrg(char *Low, char *Upp, int cnt); ... whitespace-delimited string. ... You're also assuming that the representations of the characters ...
    (comp.lang.c)
  • Re: A string collection abstract data type
    ... duplicate string in Insert, InsertAt, ReplaceAt; ... used int instead of size_t for count and size for consistency with API. ... char *(StringCollection *SC, int idx, ... static int IsReadOnly; ...
    (comp.lang.c)