C program using sprintf()/write() has extra characters

Question

I'm writing a C program where a user inputs 10 integers into an array, and the program outputs the reverse of that array on the console as well as writing it to a file. The only issue with my program is that in both outputs, there are extra characters. In the console, there is a "%" before the first integer. In the file I write to, there are two empty boxes in front of every integer except the first one (and also on a new line after the last integer.

#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char const *argv[]) {
  int i;
  int input;
  char *filename = "/home/privateinfo/5/output";
  char *buffer = malloc(sizeof(int));
  int *tenInts = malloc(10 * sizeof(int));

  /* open the file for writing */
  int fd = open(filename, O_WRONLY | O_TRUNC);

  if (fd < 0) {
    return -1;
  }

  /* read ten integers from keyboard and store in array*/
  write(1, "Enter 10 integers:\n", 20) ;

  for (i = 0; i < 10; i++) {
    scanf("%d", &input);
    tenInts[i] = input;
  }

  write(1, "\nThe integers reversed are:\n", 30);

  /* print array in reverse and write to file*/
  for (i = 9; i > -1; i--) {
    sprintf(buffer, "%d\n", tenInts[i]);
    write(1, buffer, sizeof(int));
    if(write(fd, buffer, sizeof(int)+1) < 0) {
      write(2, "There was an error writing to the file.\n", 50);
      return -1;
    }
  }

  /* close the file and clear pointer */
  close(fd);

  return 0;
}

Here is my output.

Answer 1

The bottom line is you fail to allocate sufficient storage for buffer and most likely invoke Undefined Behavior when you attempt to store more characters than you have room for in buffer. You additionally invoke Undefined Behavior when you attempt to write more characters to stdout than you have characters for (you are off-by-one (or more) in your count of characters in the string literals because "\n" is one-character (the newline character, ASCII 10, or 0xa), not multiple characters of '\' and 'n'.

Unfortunately, that is but the tip of the iceberg. First, as couple of notes, avoid hardcoding Magic-Numbers in your code (10 is a Magic-Number) and avoid hardcoding Filenames). If you need to change a number like 10 because you want 11 integers, look how many changes are necessary. If you want to write to a different file, you have to re-compile your code. Instead:

#define NINT     10   /* if you need a constant, #define one (or more) */
#define INTCHARS 32   /* adequate for LLONG_MIN or ULLONG_MAX (20+1 chars) */
#define FCRMODE  S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH

Now you have a single place to change a constant at the top of your code should you need to make a change.

For your filename, you can provide a default, but your program takes argument for main when it is declared as:

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

Pass the filename in as an argument to your program or use the hardcoded filename as a default - only. Example:

    char *filename = argc > 1 ? argv[1] : "/home/privateinfo/5/output";

If an argument is provided it is used as the filename, otherwise, your default is used.

In your allocation of:

char *buffer = malloc(sizeof(int));

You only allocate 4-bytes to buffer that means at maximum you can only store 3-digits and the nul-terminating character, or since you also include the '\n' character, you can store no more than a 2-digit number. Instead, there is no reason to allocate for buffer. The longest long long int or unsigned long long on a 64-bit computer is 20-characters (including the '-' minus sign) +1 for the nul-terminating character for a total of 21-characters needed. Simply define a 32-byte buffer and you can include your '\n' with 10-characters of safety, e.g.

    char buffer[INTCHARS] = "";

Next, don't count characters for write. If you a printing String-Literals, simply using strlen() will give you to proper number of characters to output. Or, if you need to create a string to write with sprintf, save the return it tells you how many characters where written to the buffer (not including the nul-terminating character), e.g.

    /* read NINT integers from keyboard and store in array*/
    nchar = sprintf (buffer, "Enter %d integers:\n", NINT);
    write (STDOUT_FILENO, buffer, nchar);       /* (or simply use printf) */

(why you are using write is a mystery when you can simply printf the string with the conversion directly to stdout)

Don't return negative values to the shell. POSIX requires that only positive values are returned. That is why the macro EXIT_FAILURE is defined as 1.

The rest remainder of the errors are just additional incorporations of what is discussed above. Putting it altogether, you can do:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>

#define NINT     10   /* if you need a constant, #define one (or more) */
#define INTCHARS 32   /* adequate for LLONG_MIN or ULLONG_MAX (20+1 chars) */
#define FCRMODE  S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH

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

    int fd, i, input, nchar, *tenInts;
    char *filename = argc > 1 ? argv[1] : "/home/privateinfo/5/output";
    char buffer[INTCHARS] = "";

    if (!(tenInts = malloc (NINT * sizeof *tenInts))) {
        perror ("malloc-tenInts");
        return 1;
    }

    /* open the file for writing */
    if ((fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT, FCRMODE)) < 0) {
        perror ("fopen-filename");
        return 1;
    }

    /* read NINT integers from keyboard and store in array*/
    nchar = sprintf (buffer, "Enter %d integers:\n", NINT);
    write (STDOUT_FILENO, buffer, nchar);       /* (or simply use printf) */

    for (i = 0; i < NINT; i++) {
        if (scanf ("%d", &input) != 1) {
            fputs ("error: invalid integer input.\n", stderr);
            return 1;
        }
        tenInts[i] = input;
    }

    write (STDOUT_FILENO, "\nThe integers reversed are:\n",
            strlen ("\nThe integers reversed are:\n"));

    /* print array in reverse and write to file*/
    for (i = NINT-1; i >= 0; i--) {
        nchar = sprintf (buffer, "%d\n", tenInts[i]);
        write (STDOUT_FILENO, buffer, nchar);
        if (write (fd, buffer, nchar) < 0) {
            write (STDERR_FILENO, "There was an error writing to the file.\n",
                    strlen ("There was an error writing to the file.\n"));
            return 1;
        }
    }

    /* ALWAYS validate close of file (after a write) */
    if (close (fd) == -1)
        perror ("close-fd");

    return 0;
}

Now you won't have any funny boxes output (a good indication of Undefined Behavior) when you run your code.

Example Use/Output

$ ./bin/tenintwrite dat/tenintwrite.txt
Enter 10 integers:
10 20 30 40 50 60 70 80 90 100

The integers reversed are:
100
90
80
70
60
50
40
30
20
10

Output File

$ cat dat/tenintwrite.txt
100
90
80
70
60
50
40
30
20
10

Look things over and let me know if you have further questions.