Working with Text Files Two

Question

A couple of questions really about the code below from which I gained assistance in a previous post.

1). Any ideas why at the end of the ouput, I get a random garbage character printed? I am freeing the files etc and checking for EOF.

2). The idea is that it can work with multiple file arguements, so I want to create new file names which increment, i.e. out[i].txt, is that possible in C?

The code itself takes a file containing words all separated by spaces, like a book for example, then loops through, and replaces each space with a \n so that it forms a list, please find the code below:

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

/*
 * 
 */
int main(int argc, char** argv) {

FILE *fpIn, *fpOut;
int i;
char c;
while(argc--) {
    for(i = 1; i <= argc; i++) {
        fpIn = fopen(argv[i], "rb");
        fpOut= fopen("tmp.out", "wb");
        while (c != EOF) {
            c = fgetc(fpIn);
            if (isspace(c)) 
                c = '\n';
            fputc(c, fpOut );
        }
    }
}
fclose(fpIn);
fclose(fpOut);
return 0;
}

Its not actually, I'm putting together a collection of word lists for Network Testing purposes, and figured running some books through this little program would create some nifty lists. — PnP, Dec 27 '11 at 20:49

Basile Starynkevitch · Accepted Answer · 2011-12-27T20:59:56.530

2

When you reach the end of file, you don't break the loop. So you are calling fputc(c, fpOut); with c==EOF which is probably an undefined behavior, or at least the writing of a \0xff byte.

And you don't call fclose inside your while(argc--) loop, so your files (except the last) are mostly never closed nor flushed.

At last, you don't test the result of fopen and you should test that it is non null (and print an error message, perhaps with something about strerror(errno) or perror, in that case).

You should have found out with a debugger (like gdb on Linux), and perhaps with the help of compiler warnings (but gcc-4.6 -Wall did not caught any bugs on your example).

You could decide that the output file name is related to input file name, perhaps with

char outname[512];
for(i = 1; i < argc; i++) {
   fpIn = fopen(argv[i], "rb");
   if (!fpIn) { perror (argv[i]); exit(1); };
   memset (outname, 0, sizeof (outname));
   snprintf (outname, sizeof(outname)-1, "%s~%d.out", argv[i], i);
   fpOut= fopen(outname, "wb");
   if (!fpOut) { perror (outname); exit(1); };
   /// etc...
   fclose(fpIn);
   fclose(fpOut);
   fpIn = fpOut = NULL;
}

edited Dec 27 '11 at 20:59

answered Dec 27 '11 at 20:47

Basile Starynkevitch

223,805
18
296
547

That aside, any ideas for Q1 in my description, thanks for the above, it was most helpful :) – PnP Dec 27 '11 at 20:52
Your garbage character is probably the `\0xff` I mentioned. On my machine, it probably is ÿ; and not `fflush`-ing or `fclose`-ing files make the output truncated arbitrarily. – Basile Starynkevitch Dec 27 '11 at 20:53
Whoopsie, I meant to say Q2, you already gave a great answer to 1 in your reply :) – PnP Dec 27 '11 at 20:54
I edited my reply to answer to Q2, which I did not understood entirely (I am only guessing what you probably mean). – Basile Starynkevitch Dec 27 '11 at 21:00
You don't need to zero-out `outname` if you're just going to call `snprintf` on the buffer anyway. Also, `snprintf` expects the size of the buffer, not the size of a writeable buffer (it will write to at most `n-1` of the string, saving the last byte for the null character). – dreamlax Dec 27 '11 at 21:09
The `memset` is not useless. It ensures the last byte is `\0` = the zero byte to terminate strings (even when `snprintf` don't have enough space) because I required `snprintf` to not consume all of the `outname` ! And it is much safer. – Basile Starynkevitch Dec 27 '11 at 22:23
@BasileStarynkevitch: Why not just set the last byte then, rather than all of them? There is no need to set them *all* to zero if `snprintf` is going to write over them anyway. Also, `snprintf` always ensures the output is null-terminated by only writing to at most `n - 1` characters. That is, if you have a buffer of size 5, it will write at most 4 bytes and save the last byte for the null character. – dreamlax Dec 29 '11 at 00:42
Agreed. Actually, I forgot that `snprintf` always add a zero bytes (while `strncpy` don't), even when reaching the maximal size. – Basile Starynkevitch Dec 29 '11 at 07:08

score 1 · Answer 2 · answered Dec 27 '11 at 20:53

Suggested changes (all untested):

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

int 
main(int argc, char** argv) {

  FILE *fpIn, *fpOut;
  int i;
  char c;
  for(i = 1; i < argc; i++) {
    fpIn = fopen(argv[i], "rb");
    if (!fpIn) {
      perror ("Unable to open input file");
      continue;
     }
    fpOut= fopen("tmp.out", "wb");
    if (!fpOut) {
      perror ("Unable to open output file");
      fclose (fpIn);
      continue;
     }
     while ((c = fgetc (fpIn)) != EOF)) {
       if (isspace(c)) 
         c = '\n';
       fputc(c, fpOut );
     }
     fclose(fpIn);
     fclose(fpOut);
  }
  return 0;
}

Working with Text Files Two

2 Answers2

Linked