Why is this crashing??

[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 );

}

[Jack Klein]

On 27 Jul 2004 21:49:03 -0700, on*********@hotmail.com (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 )
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; ^^^^^^^^^^
Uninitialized pointer to char.
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
No test to see if fopen() succeeds or fails, returning NULL.
while (line != NULL)
First pass through the loop, you are testing the value of an
uninitialized pointer variable. Undefined behavior.
{
line = fgets(drinks[count].brand,STRINGSIZE,inFile);
If the fopen() failed, you are passing a null FILE pointer to fgets().
Undefined behavior.
drinks[count].brand [strlen (drinks[count].brand) - 1] = '\0';
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);
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 );

}

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

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

[Barry Schwarz]

On 27 Jul 2004 21:49:03 -0700, on*********@hotmail.com (Jeffrey
Barrett) wrote:

/*
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;
line is currently uninitialized.
FILE* inFile;
int count = 0;
inFile = fopen ( "machine.txt", "r" );
Did the file open properly? You need to check.
while (line != NULL)
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);
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';
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++;
count is now set to 7.
}

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

}

<<Remove the del for email>>

[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) ^^^^^^^^^^^^
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 );

}

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");
}

[Emmanuel Delahaye]

Jeffrey Barrett wrote on 28/07/04 :

Why is this crashing??

<code snipped>

Is this clear enough ?

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

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html

"C is a sharp tool"

[Emmanuel Delahaye]

Emmanuel Delahaye wrote on 28/07/04 :

Jeffrey Barrett wrote on 28/07/04 :

Why is this crashing??

<code snipped>

Is this clear enough ?

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

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;
}

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html

"C is a sharp tool"

[Christopher Benson-Manica]

Jeffrey Barrett <on*********@hotmail.com> spoke thus:

typedef char String [STRINGSIZE]; String brand;

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 :)

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[Jeffrey Barrett]

Martin Ambuhl <ma*****@earthlink.net> wrote in message news:<2m************@uni-berlin.de>...

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)

^^^^^^^^^^^^
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 );

}

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");
}

worked perfect. thanks