jayward@eecs.cs.pdx.edu (Jay Ward) (03/16/91)
Hello! I am trying to convert from programming in Pascal to C
and having a rather CHALLENGING time! If anyone can help, I would
really appreciate it. I wrote the following program to split one file
that contains a bunch of shar files (pulled from comp.binaries.amiga)
into separate files (one per shar file). The program isn't even
creating the proper filenames. NOTE: I tested the function OUTVERS
separately and it seemed to work fine.
------
#include <stdio.h>
char * outvers(char *, int);
main(argc, argv)
int argc;
char *argv[];
{
FILE *infile, *outfile;
int filecount;
char b[80], *filename;
infile = fopen(argv[1], "rb");
filecount = 1;
while(fgets(b,80,infile) !=NULL)
{
filename = outvers(argv[2], filecount);
outfile = fopen(filename, "wb");
while((b[0] != '#') && (b[1] != '-'))
{
fgets(b,80,infile);/* discard unneeded lines */
}
while((b[0] != '-') && (b[1] != '-'))
{
printf("Outfile ==> %s\n", filename);
printf("%s", b);
fputs(b, outfile); /* save the good ones */
fgets(b,80,infile);
}
fclose(outfile);
filecount++;
}
fclose(infile);
}
/* outvers will return a filename with the passed name followed by a single
character extender letter that corresponds with the integer passed */
char * outvers(file, version)
char *file; /* Needs a pointer to a string */
int version; /* and an integer */
{
int i = 0;
char filename[30];
/* go to end of string */
while(file[i] != '\0') filename[i] = file[i++];
filename[i++] = '.'; /* add a version extender */
filename[i++] = 64+version;
filename[i++] = '\0';
return (char *) &filename;
}
----
Thanks in advance!
----------------------------------------------------------------------------
Jay Ward --> jayward@eecs.cs.pdx.edu | if (TrailBlazers > Opponents)
" " - Marcel Marceau | TrailBlazerWins++;
----------------------------------------------------------------------------varney@cbnewsd.att.com (Al Varney) (03/19/91)
Hi Jay! In article <1980@pdxgate.UUCP> you write: >Hello! I am trying to convert from programming in Pascal to C >and having a rather CHALLENGING time! And I did it the other way around, and had a FRUSTRATING time! >#include <stdio.h> > >char * outvers(char *, int); > >main(argc, argv) >int argc; >char *argv[]; > >{ > FILE *infile, *outfile; > int filecount; > char b[80], *filename; > > infile = fopen(argv[1], "rb"); ^^^^^^ ALWAYS check for failure... if(!infile) {something} (I realize that maybe you stripped out this stuff for Usenet.) > filecount = 1; [Shouldn't this be 0?]..... > > while(fgets(b,80,infile) !=NULL) > { > filename = outvers(argv[2], filecount); > > outfile = fopen(filename, "wb"); ^^^^^^^ Check for failure... > while((b[0] != '#') && (b[1] != '-')) Hmm, this loop will terminate as soon as either "#" or "-" appear in the appropriate place. A '(! x && ! y)' is really a '!(x || y)' test. Think of it this way: while( TRUE and TRUE ) is the only way to keep the while loop going; if either "TRUE" is really "FALSE", the loop ends. So you will stop as soon as either Column 1 = '#' OR Column 2 = '-'. Sheesh, "Column"s -- is my age showing??? The typical C idiom for waiting on "#-" would be: while((b[0] != '#') || (b[1] != '-')) Or some folks like: while( ! ((b[0] == '#') && (b[1] == '-'))) > { > fgets(b,80,infile);/* discard unneeded lines */ > } > while((b[0] != '-') && (b[1] != '-')) Same "while" problem. Also, add an End-of-File check for "infile"... After all, some folks try to emulate the "shar" format by hand, and might miss the magic "----" ending at the end of the file. Say... while( ((b[0] != '-') || (b[1] != '-')) && !feof(infile)) > { > printf("Outfile ==> %s\n", filename); > printf("%s", b); Do you really want to print this for every line read? > fputs(b, outfile); /* save the good ones */ > fgets(b,80,infile); Here's an alternative good place to put an "feof(infile)" check, like... if(feof(infile)) break; /* Why read garbage, eh? */ > } > fclose(outfile); > filecount++; Also need to check "feof()" here or on the outer while loop. if(feof(infile)) break; > } > fclose(infile); >} > >/* outvers will return a filename with the passed name followed by a single >character extender letter that corresponds with the integer passed */ > >char * outvers(file, version) >char *file; /* Needs a pointer to a string */ >int version; /* and an integer */ > >{ > int i = 0; > char filename[30]; This is the "real" culprit in your problem. This statement says to allocate 30 characters of "auto" or "automatic" space for the name 'filename', which is known only to this function. THE SPACE IS RELEASED WHEN THE FUNCTION TERMINATES. See comments near the "return" below... > > > /* go to end of string */ > while(file[i] != '\0') filename[i] = file[i++]; Woops, 3 problems here: 1) NEVER trust your input, so check for exceeding the filename[], 2) "i" is never set back to 0 after the first time through the loop, 3) NEVER use "autoincrement" or any assignment statement when the incremented or assigned-to variable is referenced elsewhere in the same statement. The C reference book: "The C Programming Language" orig. edition, by Kernighan and Ritchie alludes to this on page 50 by stating that: a[i] = i++; yields different answers on different compilers. Same holds for function call order, function parameter evaluation and nested assignment statements like: filename[i] = file[ i = i + 1 ]; Generic term is "side-effects", and the compiler controls when. (Unfortunately, the index to the reference book doesn't show page 50 when you look up the "increment" operator, but do look at the references to "evaluation", it's an eye-opener.) Even putting parentheses around things doesn't do the trick of forcing the order of reference to the same variable in the same statement. And the new ANSI/ISO Standard C doesn't allow you to force the order here, either. At least I don't see how the "+" prefix operator would help here. Fortunately, when a variable is referenced more than once in a single statement, you should take the hint that there might be a better way to write the statement(s): {filename[i] = file[i]; i++;} controls the sequence and is just as fast when compiled. or a change in control might fix all 3 problems: for( i=0; file[i] != '\0' && i < 27; i++) filename[i] = file[i]; > > filename[i++] = '.'; /* add a version extender */ > filename[i++] = 64+version; Most C books show you how to avoid memorizing the ASCII char. set by filename[i++] = '0' + version; > filename[i++] = '\0'; > > > return (char *) &filename; So, you return a POINTER to an "automatic" variable, and then leave the scene, eh? The little grey cells tell me that by the time you reference the returned POINTER in "main", the memory area will be wiped out with other "automatic" variables used in other functions. Naughty, naughty. >} >Jay Ward Welcome to a real language :-) <-a smile here. "Any programming language powerfull enough to be really useful is bound to be a challenge, just like people languages." -- I just made that up, so maybe it's not a quote until someone else quotes me!! Am I allowed to quote myself? To show you've really reached the "C" world, change the program to use POINTERS every that you have used array references, except for the "filename[]-type" declarations of course. Da disclaimer: These are not AT&T's words, and I only program in C a few times a year, so there's probably a few errors, and it's early, and .... Al Varney, AT&T, att!ihlpf!varney