Why is this crashing??

J

Jeffrey Barrett

/*
machine.txt:
------------------
Cola
0.75 20
Ruby Red Blast
1.00 10
Lemon Fizz
0.75 8
Grape Soda
0.90 5
Citrus Flip
0.85 0
Habanero Surprise
0.80 11
-------------------
*/



/* ================================================================ */

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

/* ================================================================ */

#define MAXNUMDRINKS 6
#define STRINGSIZE 25

/* ================================================================ */

typedef char String [STRINGSIZE];
typedef enum bool { false, true } bool;

typedef struct drinkRec
{
String brand;
float price;
int quantity;
bool soldOut;
} drinkRec;

typedef drinkRec drinkList [ MAXNUMDRINKS ];

/* ================================================================ */

void ReadAllDrinks ( int* numDrinks, drinkList drinks );

/* ================================================================ */

int main ( )
{
drinkList drinks;
int numDrinks;
printf ("Welcome to the Soda Machine.\n\n");
ReadAllDrinks ( &numDrinks, drinks );
return ( 0 );

}
/* ================================================================ */

void ReadAllDrinks (int* numDrinks, drinkList drinks )

{
String dummy;
char *line;
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
while (line != NULL)
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
fscanf(inFile,"%f%d",&drinks[count].price,&drinks[count].quantity);
fgets(dummy,STRINGSIZE,inFile);
count++;
}

*numDrinks = (count - 1);
fclose ( inFile );

}
 
J

Jack Klein

On 27 Jul 2004 21:49:03 -0700, (e-mail address removed) (Jeffrey
Barrett) wrote in comp.lang.c:

What have you done to test it? Run it in a debugger? Added printf()
statements inside the ReadAllDrinks() function to see how far it gets?

And what do you mean by "crashing"? Exactly what happens when you
build and/or run the program?

I am not going to do an exhaustive analysis, but I will point out some
errors.
/*
machine.txt:
------------------
Cola
0.75 20
Ruby Red Blast
1.00 10
Lemon Fizz
0.75 8
Grape Soda
0.90 5
Citrus Flip
0.85 0
Habanero Surprise
0.80 11
-------------------
*/



/* ================================================================ */

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

/* ================================================================ */

#define MAXNUMDRINKS 6
#define STRINGSIZE 25

/* ================================================================ */

typedef char String [STRINGSIZE];
typedef enum bool { false, true } bool;

typedef struct drinkRec
{
String brand;
float price;
int quantity;
bool soldOut;
} drinkRec;

typedef drinkRec drinkList [ MAXNUMDRINKS ];

/* ================================================================ */

void ReadAllDrinks ( int* numDrinks, drinkList drinks );

/* ================================================================ */

int main ( )
{
drinkList drinks;
int numDrinks;
printf ("Welcome to the Soda Machine.\n\n");
ReadAllDrinks ( &numDrinks, drinks );
return ( 0 );

}
/* ================================================================ */

void ReadAllDrinks (int* numDrinks, drinkList drinks )
Click to expand...

Why have a function with a void return type accept a pointer to a
value in the caller to store its result? Why not just have
ReadAllDrinks return an int with the number of records read
successfully?
{
String dummy;
char *line;
Click to expand...
^^^^^^^^^^
Uninitialized pointer to char.
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
Click to expand...

No test to see if fopen() succeeds or fails, returning NULL.
while (line != NULL)
Click to expand...

First pass through the loop, you are testing the value of an
uninitialized pointer variable. Undefined behavior.
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
Click to expand...

If the fopen() failed, you are passing a null FILE pointer to fgets().
Undefined behavior.
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
Click to expand...

If there are too many characters in the input line, the one you are
overwriting with '\0' won't be a newline. See
http://www.jk-technology.com/ctips01.html#safe_gets, which shows how
to remove the newline from a string read by fgets() if and only if
there is actually one there.
fscanf(inFile,"%f%d",&drinks[count].price,&drinks[count].quantity);
Click to expand...

No test on the return value of fscanf() to see if it succeeded or
failed.
fgets(dummy,STRINGSIZE,inFile);
count++;
}

*numDrinks = (count - 1);
fclose ( inFile );

}
Click to expand...

Deal with the issues above and describe exactly what happens, not just
"crashing".
 
B

Barry Schwarz

/*
machine.txt:
------------------
Cola
0.75 20
Ruby Red Blast
1.00 10
Lemon Fizz
0.75 8
Grape Soda
0.90 5
Citrus Flip
0.85 0
Habanero Surprise
0.80 11
-------------------
*/



/* ================================================================ */

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

/* ================================================================ */

#define MAXNUMDRINKS 6
#define STRINGSIZE 25

/* ================================================================ */

typedef char String [STRINGSIZE];
typedef enum bool { false, true } bool;

typedef struct drinkRec
{
String brand;
float price;
int quantity;
bool soldOut;
} drinkRec;

typedef drinkRec drinkList [ MAXNUMDRINKS ];

/* ================================================================ */

void ReadAllDrinks ( int* numDrinks, drinkList drinks );

/* ================================================================ */

int main ( )
{
drinkList drinks;
int numDrinks;
printf ("Welcome to the Soda Machine.\n\n");
ReadAllDrinks ( &numDrinks, drinks );
return ( 0 );

}
/* ================================================================ */

void ReadAllDrinks (int* numDrinks, drinkList drinks )

{
String dummy;
char *line;
Click to expand...

line is currently uninitialized.
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
Click to expand...

Did the file open properly? You need to check.
while (line != NULL)
Click to expand...

The first time this statement executes, you invoke undefined behavior
because line is not initialized. If line were set to NULL by
accident, the rest of the function would not execute.
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
Click to expand...

The last time through this loop (after reading the 0.80 and 11), fgets
will fail (and line will be set to NULL).
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
Click to expand...

But count is now 6 and drinks[6] does not exist. You are again
invoking undefined behavior and probably overwriting critical parts of
your program.
fscanf(inFile,"%f%d",&drinks[count].price,&drinks[count].quantity);
Ditto

fgets(dummy,STRINGSIZE,inFile);
count++;
Click to expand...

count is now set to 7.
}

*numDrinks = (count - 1);
fclose ( inFile );

}
Click to expand...



<<Remove the del for email>>
 
M

Martin Ambuhl

Jeffrey Barrett wrote (hidden is subject: "Why is this crashing??"):

[...]
void ReadAllDrinks (int* numDrinks, drinkList drinks )

{
String dummy;
char *line;
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
while (line != NULL)
Click to expand...
^^^^^^^^^^^^
line is not initialized here
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
fscanf(inFile,"%f%d",&drinks[count].price,&drinks[count].quantity);
fgets(dummy,STRINGSIZE,inFile);
count++;
}

*numDrinks = (count - 1);
fclose ( inFile );

}
Click to expand...

See if this works better for you:

void ReadAllDrinks(int *numDrinks, drinkList drinks)
{
String dummy;
char *line;
FILE *inFile;
int count = 0;
if ((inFile = fopen("machine.txt", "r"))) {
while ((line = fgets(drinks[count].brand, STRINGSIZE, inFile))) {
drinks[count].brand[strlen(drinks[count].brand) - 1] = '\0';
fscanf(inFile, "%f%d", &drinks[count].price,
&drinks[count].quantity);
fgets(dummy, STRINGSIZE, inFile);
count++;
}
*numDrinks = (count - 1);
fclose(inFile);
}
else fprintf(stderr, "machine.txt not read\n");
}
 
E

Emmanuel Delahaye

Jeffrey Barrett wrote on 28/07/04 :
Why is this crashing??
Click to expand...

<code snipped>

Is this clear enough ?

Compiling MAIN.C:
Warning MAIN.C 52: Possible use of 'line' before definition
Linking EXE\PROJ.EXE:
 
E

Emmanuel Delahaye

Emmanuel Delahaye wrote on 28/07/04 :
Jeffrey Barrett wrote on 28/07/04 :


<code snipped>

Is this clear enough ?

Compiling MAIN.C:
Warning MAIN.C 52: Possible use of 'line' before definition
Linking EXE\PROJ.EXE:
Click to expand...

Try that

/* ================================================================ */

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

#ifdef __BORLANDC__
/* The pesky "floating point formats not linked" killer hack : */
extern unsigned _floatconvert;
#pragma extref _floatconvert
#endif

/* ================================================================ */

typedef char String[25];

typedef struct drinkRec
{
String brand;
float price;
int quantity;
unsigned soldOut:1;
}
drinkRec;

/* ================================================================ */

static void ReadAllDrinks (int *numDrinks, drinkRec * drinks, size_t
nb)
{
String dummy;
int count = 0;
FILE *inFile = fopen ("machine.txt", "r");

if (inFile != NULL)
{
int first = 1;
drinkRec *p = NULL;

while (fgets (dummy, sizeof dummy, inFile) != NULL)
{
/* clean the read line */
{
char *p = strchr (dummy, '\n');

if (p)
{
*p = 0;
}
else
{
int c;

while ((c = fgetc (inFile)) != '\n' && c != EOF)
{
}
fprintf (stderr, "Line is too long\n");
break;
}
}

assert (count < nb);

if (first)
{
p = drinks + count;

/* clean the record */
{
static const drinkRec z =
{
{0}
};
*p = z;
}

strcpy (p->brand, dummy);
first = 0;
}
else
{
assert (p != NULL);
{
int n = sscanf (dummy, "%f%d", &p->price, &p->quantity);
if (n != 2)
{
fprintf (stderr, "Data error\n");
break;
}
}
first = 1;
count++;
}

}
fclose (inFile), inFile = NULL;

if (numDrinks != NULL)
{
*numDrinks = count;
}
}
}

static void PrintAllDrinks (int numDrinks, drinkRec const *drinks,
size_t nb)
{
printf ("Choice of %d drink%s\n"
,numDrinks
,numDrinks > 1 ? "s" : "");

{
size_t i;

for (i = 0; i < numDrinks; i++)
{
drinkRec const *const p = drinks + i;

assert (i < nb);

printf ("%-25s %5.2f %3d %1d\n"
,p->brand
,p->price
,p->quantity
,(int) p->soldOut);
}
}

}

/* ================================================================ */

int main (void)
{
drinkRec drinks[6];
int numDrinks;

printf ("Welcome to the Soda Machine.\n\n");
ReadAllDrinks (&numDrinks, drinks, sizeof drinks / sizeof *drinks);
PrintAllDrinks (numDrinks, drinks, sizeof drinks / sizeof *drinks);

return 0;
}
 
C

Christopher Benson-Manica

Jeffrey Barrett said:
typedef char String [STRINGSIZE];
Click to expand...
String brand;
Click to expand...

I don't know if I'm the only one, but I found this String declaration
to be quite misleading - if the reader happens to miss the typedef,
he's going to wonder what the hell is going on :)
 
J

Jeffrey Barrett

Martin Ambuhl said:
Jeffrey Barrett wrote (hidden is subject: "Why is this crashing??"):

[...]
void ReadAllDrinks (int* numDrinks, drinkList drinks )

{
String dummy;
char *line;
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
while (line != NULL)
Click to expand...
^^^^^^^^^^^^
line is not initialized here
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
fscanf(inFile,"%f%d",&drinks[count].price,&drinks[count].quantity);
fgets(dummy,STRINGSIZE,inFile);
count++;
}

*numDrinks = (count - 1);
fclose ( inFile );

}
Click to expand...

See if this works better for you:

void ReadAllDrinks(int *numDrinks, drinkList drinks)
{
String dummy;
char *line;
FILE *inFile;
int count = 0;
if ((inFile = fopen("machine.txt", "r"))) {
while ((line = fgets(drinks[count].brand, STRINGSIZE, inFile))) {
drinks[count].brand[strlen(drinks[count].brand) - 1] = '\0';
fscanf(inFile, "%f%d", &drinks[count].price,
&drinks[count].quantity);
fgets(dummy, STRINGSIZE, inFile);
count++;
}
*numDrinks = (count - 1);
fclose(inFile);
}
else fprintf(stderr, "machine.txt not read\n");
}
Click to expand...

worked perfect. thanks
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

quick help for weird problem. 4
Chatbot 0
TF-IDF 2
why is this map crashing? 6
why this program is crashing 6
Q: Newbee: Why is this little program not working 11
Is this a functor? 2
pointer changes value between calls, why/how? 6

Members online

Total: 2,601 (members: 1, guests: 2,600)
Robots: 115

Forum statistics

Threads
474,145
Messages
2,570,826
Members
47,371
Latest member
Brkaa

Latest Threads

Top