Redirecting input from file

D

dmoran21

Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.

Thanks,
Dave

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

int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
FILE *fpin;
FILE *fpout;

fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");

/* Array Allocation */
precipvals = (float*)malloc(sizeof(int) * arraylength);
precip3hrs = (float*)malloc(sizeof(int) * arraylength);
date = (long*)malloc(sizeof(int) * arraylength);

/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%l,%f", &date[count], &precipvals[count]);
if(feof(fpin)) {
fclose(fpin);
return -3;
}
if(count == 1) {
precip3hrs[count] = precipvals[count];
}
if(count == 2) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 2) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
}

/* Output section */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count == 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
fclose(fpin);
fclose(fpout);

/* Deallocation */
free(date);
date = (long*)NULL;
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
return 0;
}
 
A

AG

Hi Dave, some hints below :
Dave

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

int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
FILE *fpin;
FILE *fpout;

fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");

/* Array Allocation */
precipvals = (float*)malloc(sizeof(int) * arraylength);
precip3hrs = (float*)malloc(sizeof(int) * arraylength);
date = (long*)malloc(sizeof(int) * arraylength);
Click to expand...
check if malloc returned a NULL pointer.
/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%l,%f", &date[count], &precipvals[count]);
Click to expand...
Could you show us the file ? It seems that count is never incremented
and always equal 1 ?
if(feof(fpin)) {
fclose(fpin);
return -3;
}
if(count == 1) {
precip3hrs[count] = precipvals[count];
}
if(count == 2) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 2) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
}

/* Output section */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
Click to expand...
If count == arraylength, then you do an overflow. You can write inside
precipvals using index [0 to arrayval-1].
if(count == 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
Click to expand...
same comment about overflows.
if(count == 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
fclose(fpin);
fclose(fpout);

/* Deallocation */
free(date);
date = (long*)NULL;
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
return 0;
}
Click to expand...
 
E

Eric Sosman

Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.
Click to expand...

There are several problems, and perhaps more than I've
spotted in a quick scan. I've noted a few of them below:
#include <stdio.h>
#include <stdlib.h>
#include <math.h>

int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
Click to expand...

The casts are harmless, but unnecessary.
FILE *fpin;
FILE *fpout;

fpin = fopen(argv[1],"r+");
Click to expand...

Why "r+" instead of just "r", since all you do is
read from the file? Also, you should check the values
returned by both fopen() calls to see if fopen() failed.
fpout = fopen(argv[2],"w");

/* Array Allocation */
precipvals = (float*)malloc(sizeof(int) * arraylength);
precip3hrs = (float*)malloc(sizeof(int) * arraylength);
date = (long*)malloc(sizeof(int) * arraylength);
Click to expand...

More unnecessary but harmless casts. A worse problem
is that you don't check any of the malloc() calls for
failure. Worst of all is that you request space for
so-and-so many ints, but the things you're actually going
to store are floats and longs!
/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%l,%f", &date[count], &precipvals[count]);
if(feof(fpin)) {
fclose(fpin);
return -3;
Click to expand...

That's a pretty weird exit value. It may mean something
on your system, but if it does the meaning is far from
universal. The only "portable" exit values are EXIT_SUCCESS
and zero (both meaning "success") and EXIT_FAILURE.
}
if(count == 1) {
precip3hrs[count] = precipvals[count];
}
if(count == 2) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 2) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
Click to expand...

Since count was initialized to zero and there's nothing
to change its value, this loop just reads and reads and reads
and does nothing. Also, if fscanf() ever runs across something
it can't interpret, it'll return the number of items it was
able to convert (zero or one) and leave the unintelligible
junk still un-read. On the next call it will still be unable
to convert the junk, so it will return zero and leave the junk
un-read. On the next call it will still be unable ...

The upshot: If there's anything wrong anywhere in the input,
this loop will never finish. If the input data is perfectly
formatted, this loop will read all the way through the file
and do essentially nothing. When (if) it reaches the end of
the input, feof(pin) will return true and the program will
exit.
}
Click to expand...

As far as I can see, there's no way for the program to
arrive at this point. But if it did ...
/* Output section */
for(count = 1; count <= arraylength; count++) {
Click to expand...

On the final trip through this loop, count will equal
arraylength and you'll try to use the [arraylength] elements
of your arrays. Since you've only allocated space for [0]
through [arraylength-1] (if that; see above), that'll be
a problem.

Also, the only array elements that have ever had anything
stored in them are the [0] elements of date and precipvals.
Nothing has ever been stored in the higher-numbered elements
of those two arrays, nor in any element of precip3hrs.

Finally, assuming you fix up the input loop so it actually
does something useful, what happens if the input has 3000
lines instead of 5000? (Or worse: What if it has 6000?)
Do you really want to loop through the entire array, or just
through the array elements that correspond to inputs?
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count == 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count <= arraylength; count++) {
Click to expand...

Same remarks as for the previous loop.
if(count == 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
fclose(fpin);
fclose(fpout);
Click to expand...

Believe it or not, fclose() can fail. Failure of the
first one probably doesn't hurt your program any, but if
the second fails it very likely means that some part of the
output file didn't get written. Do you want to proclaim
"Success!" if that happens?
/* Deallocation */
free(date);
date = (long*)NULL;
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
Click to expand...

More useless casts. (Useless assignments, too, IMHO.)
 
R

Richard Heathfield

(e-mail address removed) said:
Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.
Click to expand...

I've made some obvious corrections - removed all the casts, added a bit
of error checking, fixed a couple of broken loop bounds...

But I can't test because I don't have the data that you have.

So here's the semi-corrected code. I'm not saying it fixes your problem,
but you might find that it at least prints out a complaint at runtime,
in which case it would be useful to know which complaint it prints.

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

int main(int argc,
char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
int count = 0;
int condition = 1;
float *precipvals = NULL;
float *precip3hrs = NULL;
long *date = NULL;
FILE *fpin;
FILE *fpout;

if(argc > 2)
{
fpin = fopen(argv[1], "r+");
if(fpin != NULL)
{
fpout = fopen(argv[2], "w");
if(fpout != NULL)
{
/* Array Allocation */
precipvals = malloc(arraylength *
sizeof *precipvals);
if(precipvals != NULL)
{
precip3hrs = malloc(arraylength *
sizeof *precip3hrs);
if(precip3hrs != NULL)
{
date = malloc(arraylength *
sizeof *date);
if(date != NULL)
{
/* Calculation Section */
while(condition != EOF)
{
condition =
fscanf(fpin,
"%ld,%f",
&date[count],
&precipvals[count]);
if(feof(fpin))
{
fclose(fpin);
fprintf(stderr,
"Last datum read, program closing now.\n");
return EXIT_FAILURE;
}
if(count == 1)
{
precip3hrs[count] = precipvals[count];
}
else if(count == 2)
{
precip3hrs[count] =
precipvals[count] + precip3hrs[count - 1];
}
else if(count > 2)
{
precip3hrs[count] =
precipvals[count] + precipvals[count - 1] +
precipvals[count - 2];
}
}

/* Output section */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count == 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count > 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count == 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count > 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}
free(date);
}
else
{
fprintf(stderr, "Allocation failure: date\n");
}
free(precip3hrs);
}
else
{
fprintf(stderr, "Allocation failure: precip3hrs\n");
}
free(precipvals);
}
else
{
fprintf(stderr, "Allocation failure: precipvals\n");
}
fclose(fpout);
}
else
{
fprintf(stderr, "Can't open %s for writing.\n", argv[2]);
}
fclose(fpin);
}
else
{
fprintf(stderr, "Can't open %s for reading\n", argv[1]);
}
}
else
{
fprintf(stderr, "Need more args.\n");
}

return 0;
}
 
D

dmoran21

(e-mail address removed) said:
Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.
Click to expand...

I've made some obvious corrections - removed all the casts, added a bit
of error checking, fixed a couple of broken loop bounds...

But I can't test because I don't have the data that you have.

So here's the semi-corrected code. I'm not saying it fixes your problem,
but you might find that it at least prints out a complaint at runtime,
in which case it would be useful to know which complaint it prints.

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

int main(int argc,
char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
int count = 0;
int condition = 1;
float *precipvals = NULL;
float *precip3hrs = NULL;
long *date = NULL;
FILE *fpin;
FILE *fpout;

if(argc > 2)
{
fpin = fopen(argv[1], "r+");
if(fpin != NULL)
{
fpout = fopen(argv[2], "w");
if(fpout != NULL)
{
/* Array Allocation */
precipvals = malloc(arraylength *
sizeof *precipvals);
if(precipvals != NULL)
{
precip3hrs = malloc(arraylength *
sizeof *precip3hrs);
if(precip3hrs != NULL)
{
date = malloc(arraylength *
sizeof *date);
if(date != NULL)
{
/* Calculation Section */
while(condition != EOF)
{
condition =
fscanf(fpin,
"%ld,%f",
&date[count],
&precipvals[count]);
if(feof(fpin))
{
fclose(fpin);
fprintf(stderr,
"Last datum read, program closing now.\n");
return EXIT_FAILURE;
}
if(count == 1)
{
precip3hrs[count] = precipvals[count];
}
else if(count == 2)
{
precip3hrs[count] =
precipvals[count] + precip3hrs[count - 1];
}
else if(count > 2)
{
precip3hrs[count] =
precipvals[count] + precipvals[count - 1] +
precipvals[count - 2];
}
}

/* Output section */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count == 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count > 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count == 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count > 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}
free(date);
}
else
{
fprintf(stderr, "Allocation failure: date\n");
}
free(precip3hrs);
}
else
{
fprintf(stderr, "Allocation failure: precip3hrs\n");
}
free(precipvals);
}
else
{
fprintf(stderr, "Allocation failure: precipvals\n");
}
fclose(fpout);
}
else
{
fprintf(stderr, "Can't open %s for writing.\n", argv[2]);
}
fclose(fpin);
}
else
{
fprintf(stderr, "Can't open %s for reading\n", argv[1]);
}
}
else
{
fprintf(stderr, "Need more args.\n");
}

return 0;

}

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Click to expand...


Thanks for the help. I'll give it a go.

Dave
 
R

Richard Heathfield

(e-mail address removed) said:
Thanks for the help. I'll give it a go.
Click to expand...

When you do, take note of the comments others have made in this thread,
especially regarding the loop counter! You'll need to fix that before
you can find the other bugs.
 
D

dmoran21

Hi Dave, some hints below :
Dave
Click to expand...
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
Click to expand...
int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
FILE *fpin;
FILE *fpout;
Click to expand...
fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");
Click to expand...
/* Array Allocation */
precipvals = (float*)malloc(sizeof(int) * arraylength);
precip3hrs = (float*)malloc(sizeof(int) * arraylength);
date = (long*)malloc(sizeof(int) * arraylength);
Click to expand...

check if malloc returned a NULL pointer.


/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%l,%f", &date[count], &precipvals[count]);
Click to expand...

Could you show us the file ? It seems that count is never incremented
and always equal 1 ?
Click to expand...

I fixed the increment. I discovered this when my boss and I were
looking at it today in my office.
if(feof(fpin)) {
fclose(fpin);
return -3;
}
if(count == 1) {
precip3hrs[count] = precipvals[count];
}
if(count == 2) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 2) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
}
Click to expand...
/* Output section */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
Click to expand...

If count == arraylength, then you do an overflow. You can write inside
precipvals using index [0 to arrayval-1].


if(count == 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
printf("One hour precipitation: %2.2f Three hour precipitation %2.2f
\n",precipvals[count],precip3hrs[count]);
}
}
Click to expand...
/* Write to file */
for(count = 1; count <= arraylength; count++) {
if(count == 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
Click to expand...

same comment about overflows.
Click to expand...

That's what I thought. My co worker said I needed the <=, I originally
just had <
if(count == 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 2) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
fclose(fpin);
fclose(fpout);
Click to expand...
/* Deallocation */
free(date);
date = (long*)NULL;
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
return 0;
}
Click to expand...
Click to expand...

Thanks for all the tips. I'm at home now, but will check it out when I
get to the office in the morning.

Dave
 
R

Roland Pibinger

On Thu, 28 Jun 2007 16:29:55 +0000, Richard Heathfield wrote:

if(argc > 2) ...
if(fpin != NULL) ...
if(fpout != NULL) ...
if(precipvals != NULL) ...
if(precip3hrs != NULL) ...
if(date != NULL) ...
while(condition != EOF) ...
if(feof(fpin)) ...
if(count == 1) ...
else if(count == 2) ...
else if(count > 2) ...
for(count = 1; count < arraylength; count++) ...
if(count == 1) ...
else if(count == 2) ...
else if(count > 2) ...
for(count = 1; count < arraylength; count++) ...
if(count == 1) ...
if(count == 2) ...
if(count > 2) ...
else ...
else ...
else ...
else ...
else ...
else ...

Are you serious? 160 lines of nested if/else and loops in one
function?
 
R

Richard Heathfield

Roland Pibinger said:

Are you serious? 160 lines of nested if/else and loops in one
function?
Click to expand...

Minimal edits consistent with adding the error checking that the OP
lacked. I wasn't about to refactor it for him as well - he can do that
himself, surely?
 
B

Barry Schwarz

On Thu, 28 Jun 2007 09:56:02 -0700, "(e-mail address removed)"


snip
Thanks for the help. I'll give it a go.
Click to expand...

Please don't quote 200 lines just to say thanks.


Remove del for email
 
C

Chris Dollin

Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.
Click to expand...

I'm going to make style points fir which other people have other
aesthetics.
#include <stdio.h>
#include <stdlib.h>
#include <math.h>

int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
Click to expand...

(a) You don't need to cast the NULL, so you shouldn't.

(b) You're going to assign to these two variables unconditionally
below, so there's no point in assigning null to them at all,
/especially/ since you're happy with initialised declarations.

Just do

float* precipvals = malloc( sizeof(int) * arraylength );
float* precip3hrs = malloc( sizeof(int) * arraylength );

(c) You don't need the casts on the `malloc` result, so (as you
see in (b) you can [and I would say /should/] remove them.

(d) You should check that `malloc` has succeeded.
long* date = (long*)NULL;
FILE *fpin;
FILE *fpout;

fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");
Click to expand...

(d) You might as well combine the declaration with the assignment.

(e) You should check that the `fopen`s succeed.

(f) In fact, since the files failing to fopen is death to your code
anyway, I'd suggest doing the fopens /first/, and structuring
your code like:

FILE *fpin = fopen(argv[1],"r+");
FILE *fpout = fopen(argv[2],"w");
if (fpin && fpout)
callFunctionThatDoesTheStuffWithTheFiles( fpin, fpout );
else
{
... code that handles files failing to fopen
}
if (fpin) close( fpin );
if (fpout) close( fpout );

That concentrates all the file opening-and-closing-and-complaining
in a short span of text.
/* Calculation Section */
Click to expand...

Make the (snipped) code here a function with a nice name.
/* Output section */
Ditto.

/* Write to file */
Click to expand...

Ditto.
/* Deallocation */
free(date);
date = (long*)NULL;
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
Click to expand...

The assignments of null are unnecessary and distracting.
 
A

Army1987

Richard Heathfield said:
(e-mail address removed) said:
Hi All, I am working on a program to take input from a txt file, do
some calculations, and then output the results to another txt file.
The program that I've written compiles fine for me, however, when I
run it, it stalls and does nothing. I'm wondering if there's something
obvious that I'm missing. My code is below and any help would be
appreciated.
Click to expand...

I've made some obvious corrections - removed all the casts, added a bit
of error checking, fixed a couple of broken loop bounds...

But I can't test because I don't have the data that you have.

So here's the semi-corrected code. I'm not saying it fixes your problem,
but you might find that it at least prints out a complaint at runtime,
in which case it would be useful to know which complaint it prints.

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

int main(int argc,
char *argv[])
{
/* Variable Declarations */
const int arraylength = 5000;
int count = 0;
int condition = 1;
float *precipvals = NULL;
float *precip3hrs = NULL;
long *date = NULL;
FILE *fpin;
FILE *fpout;

if(argc > 2)
{
fpin = fopen(argv[1], "r+");
if(fpin != NULL)
{
fpout = fopen(argv[2], "w");
if(fpout != NULL)
{
/* Array Allocation */
precipvals = malloc(arraylength *
sizeof *precipvals);
if(precipvals != NULL)
{
precip3hrs = malloc(arraylength *
sizeof *precip3hrs);
if(precip3hrs != NULL)
{
date = malloc(arraylength *
sizeof *date);
if(date != NULL)
{
/* Calculation Section */
while(condition != EOF)
{
condition =
fscanf(fpin,
"%ld,%f",
&date[count],
&precipvals[count]);
if(feof(fpin))
{
fclose(fpin);
fprintf(stderr,
"Last datum read, program closing now.\n");
return EXIT_FAILURE;
}
if(count == 1)
{
precip3hrs[count] = precipvals[count];
}
else if(count == 2)
{
precip3hrs[count] =
precipvals[count] + precip3hrs[count - 1];
}
else if(count > 2)
{
precip3hrs[count] =
precipvals[count] + precipvals[count - 1] +
precipvals[count - 2];
}
}

/* Output section */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count == 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
else if(count > 2)
{
printf
("One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}

/* Write to file */
for(count = 1; count < arraylength; count++)
{
if(count == 1)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count == 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
if(count > 2)
{
fprintf(fpout,
"One hour precipitation: %2.2f"
" Three hour precipitation %2.2f\n",
precipvals[count],
precip3hrs[count]);
}
}
free(date);
}
else
{
fprintf(stderr, "Allocation failure: date\n");
}
free(precip3hrs);
}
else
{
fprintf(stderr, "Allocation failure: precip3hrs\n");
}
free(precipvals);
}
else
{
fprintf(stderr, "Allocation failure: precipvals\n");
}
fclose(fpout);
}
else
{
fprintf(stderr, "Can't open %s for writing.\n", argv[2]);
}
fclose(fpin);
}
else
{
fprintf(stderr, "Can't open %s for reading\n", argv[1]);
}
}
else
{
fprintf(stderr, "Need more args.\n");
}

return 0;
}
Click to expand...
Apparently this will *always* return 0, no matter what happens...
Why do you think that having umpteen nested if statements is any
less bad than ever calling exit(EXIT_FAILURE)? I remember about
your double ctx2174_frob_foo(foo *f, bar *b, baz *z, quux *q), but
this isn't much clearer...
 
E

Eric Sosman

Chris Dollin wrote On 06/29/07 04:10,:
[...]

float* precipvals = malloc( sizeof(int) * arraylength );
Click to expand...
^^^^^ ^^^
float* precip3hrs = malloc( sizeof(int) * arraylength );
Click to expand...
^^^^^ ^^^

Avoiding this error is one of the chief virtues of
the c.l.c. Orthodox Allocation Idiom:

AnyType *ptr = malloc(sizeof *ptr * arraylength);
 
R

Richard Heathfield

Army1987 said:

Apparently this will *always* return 0, no matter what happens...
Click to expand...

Yeah. I said I fixed *some* of his problems, not all of them. The code
needs refactoring[1]. I am prepared to spend a considerable amount of
my time helping people out, for free, but I get to decide where I stop.
And on this occasion I chose to stop at the point where I'd stripped
out pointless casts, added a small handful of tests, and put the
free()s and fclose()s in the right places. Oh, and I corrected a couple
of loop termination conditions.

I didn't look closer at the code than that, but then I didn't claim that
I'd fixed his problem, either.

I did suggest that this might give better diagnostic information (which
is true in the long term, but it turns out that he had loop counter
problems, among other things). In a later reply, having read the loop
counter replies of others, I pointed the OP to them.

If you want the code re-factored, *you* do it.


[1] In fact, to be brutally honest, the code wants discarding and
rewriting from scratch by someone more experienced with C.
 
A

Army1987

Richard Heathfield said:
Army1987 said:

Apparently this will *always* return 0, no matter what happens...
Click to expand...

Yeah. I said I fixed *some* of his problems, not all of them. The code
needs refactoring[1]. I am prepared to spend a considerable amount of
my time helping people out, for free, but I get to decide where I stop.
And on this occasion I chose to stop at the point where I'd stripped
out pointless casts, added a small handful of tests, and put the
free()s and fclose()s in the right places. Oh, and I corrected a couple
of loop termination conditions.

I didn't look closer at the code than that, but then I didn't claim that
I'd fixed his problem, either.

I did suggest that this might give better diagnostic information (which
is true in the long term, but it turns out that he had loop counter
problems, among other things). In a later reply, having read the loop
counter replies of others, I pointed the OP to them.

If you want the code re-factored, *you* do it.
Click to expand...
Do you mean that, when the OP wrote:
fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");
you found it easier to write that code than simplily to add

fpin = fopen(argv[1], "r+");
if (fpin == NULL) {
fprintf(stderr, "Cannot open '%s': %s\n", argv[1],
strerror(errrno));
exit(EXIT_FAILURE);
}
fpout = fopen(argv[2],"w");

if (fpout == NULL) {
fprintf(stderr, "Cannot open or create '%s': %s\n", argv[2],
strerror(errrno));
fclose(fpin);
exit(EXIT_FAILURE);
}
 
R

Richard Heathfield

Army1987 said:
Richard Heathfield said:
Army1987 said:

Apparently this will *always* return 0, no matter what happens...
Click to expand...

Yeah. I said I fixed *some* of his problems, not all of them. The
code needs refactoring[1]. I am prepared to spend a considerable
amount of my time helping people out, for free, but I get to decide
where I stop. And on this occasion I chose to stop at the point where
I'd stripped out pointless casts, added a small handful of tests, and
put the free()s and fclose()s in the right places. Oh, and I
corrected a couple of loop termination conditions.

I didn't look closer at the code than that, but then I didn't claim
that I'd fixed his problem, either.

I did suggest that this might give better diagnostic information
(which is true in the long term, but it turns out that he had loop
counter problems, among other things). In a later reply, having read
the loop counter replies of others, I pointed the OP to them.

If you want the code re-factored, *you* do it.
Click to expand...
Do you mean that, when the OP wrote:
fpin = fopen(argv[1],"r+");
fpout = fopen(argv[2],"w");
you found it easier to write that code than simplily to add

fpin = fopen(argv[1], "r+");
if (fpin == NULL) {
fprintf(stderr, "Cannot open '%s': %s\n", argv[1],
strerror(errrno));
exit(EXIT_FAILURE);
}
fpout = fopen(argv[2],"w");

if (fpout == NULL) {
fprintf(stderr, "Cannot open or create '%s': %s\n", argv[2],
strerror(errrno));
fclose(fpin);
exit(EXIT_FAILURE);
}
Click to expand...

I'm saying that I found it easier to add this code:

if(fpin != NULL)
{

and this code:

fclose(fpin);
}
else
{
fprintf(stderr, "Can't open %s for reading\n", argv[1]);
}

....do much the same for fpout, and then run the result through indent.

I reckon my solution involves less typing than yours. :)
 
D

dmoran21

I've got one more error that I can't seem to get worked out. I have a
segmentation fault and upon playing with my code, I think that it
crashes when I try to deallocate the "date" array.

Here's the most recent version of my code. Everything else seems to
work properly.

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

int main(int argc, char *argv[])
{
/* Variable Declarations */
const int arraylength = 5;
const int errorcode = -1;
float sum = 0;
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
FILE *fpin;
FILE *fpout;

fpin = fopen(argv[1],"r");
fpout = fopen(argv[2],"w");

/* Array Allocation */
precipvals = (float*)malloc(sizeof(int) * arraylength);
precip3hrs = (float*)malloc(sizeof(int) * arraylength);
date = (long*)malloc(sizeof(int) * arraylength);

/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%ld,%f", &date[count],
&precipvals[count]);

if(count == 0) {
precip3hrs[count] = precipvals[count];
}
if(count == 1) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 1) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
count++;
}

/* Output section */
for(count = 0; count < arraylength; count++) {
if(count == 0) {
printf("One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 1) {
printf("One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}


/* Write to file */
for(count = 0; count < arraylength; count++) {
if(count == 0) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 1) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 1) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
fclose(fpin);
fclose(fpout);
printf("Testing\n");

/* Deallocation */
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
free(date);
date = (long*)NULL;

return 0;
}

Thanks,
Dave
 
R

Richard Heathfield

(e-mail address removed) said:
I've got one more error that I can't seem to get worked out. I have a
segmentation fault and upon playing with my code, I think that it
crashes when I try to deallocate the "date" array.

Here's the most recent version of my code. Everything else seems to
work properly.
Click to expand...

You seem to have removed all the improvements I made to your code last
time you posted the program here. To save us both time, I won't suggest
any others.

FCOL.
 
D

dmoran21

(e-mail address removed) said:



You seem to have removed all the improvements I made to your code last
time you posted the program here. To save us both time, I won't suggest
any others.

FCOL.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Click to expand...


I'm sorry. This was the code I got from my boss, not the one I was
working on. Let me see if I can find the one I was working on.

Dave
 
C

CBFalconer

[email protected] said:
.... snip ...
if(count > 1) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
count++;
}

/* Output section */
for(count = 0; count < arraylength; count++) {
if(count == 0) {
/* this continues below - cbf */ printf("One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
Click to expand...
.... more snip ...

Please do not use tabs - use spaces, 3 at a time. Otherwise you
can get the above mess, or, on some systems, all leading tabs are
deleted.
 

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

How to try a range of hex values in C# code ? 0
Drawing missing in bitmap in a pure C win32 program 4
General Protection Exception 37
Can not read VCD file in Linux 8
How can I view / open / render / display a pdf file with c code? 0
File Limit of 1021 7
trouble reading from file 6
Write to file 22

Members online

No members online now.
Total: 28 (members: 0, guests: 28)
Robots: 205

Forum statistics

Threads
473,766
Messages
2,569,569
Members
45,042
Latest member
icassiem

Latest Threads

Top