Why doesn't the following (perfectly valid) C code show the contents of a file in Objective-C?

Question

Here's a very basic C snippet to open and read a file:

  int fd = open("test.txt",O_RDONLY);
  char buf[128];
  int reader = read(fd,buf,128);
  int i;
    for (i=0;i<strlen(buf);i++)
    {
     printf("%i: I read: %c", i, (int)buf[i]);
    }

I'm also including these standard headers:

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

This code will work fine in C, and since Objective-C is a superset of C I expected it to work fine in Obj-C as well. But instead I'm getting all garbage data, such as:

0: I read: –
1: I read: *
2: I read: :

Why?

Answer 1

You shouldn't use strlen on the output buffer from read() - it isn't null-terminated.

Answer 2

Your open call is probably failing. Try specifying the full path to the file, and employ some error checking.

Answer 3

First, having the strlen() call in your for statement is a very bad practice... unless the compiler optimizes it out, your loop will run in Order N**2 time.

Second, how do you know the buffer is zero-terminated? You're making an assumption here that may not be valid. Since you know the size of the buffer (128), you should be using a constant, e.g., BUFSIZE and the conditional in the loop should be 'i < BUFSIZE'.

Third, you are casting the character value stored in buf[i] to an int, and then printing it as a char. Evidently Objective-C doesn't do this in the same way that your desktop C compiler does, and this is weakly defined in the language anyway. It would be better to just reference buf[i] without casting it to an int. I'm sure that this is where your problem is.

Answer 4

Summarizing the responses so far, and adding my own observations:

You should check the return value from open() and read(). If they fail, you're going to blindly continue on and print garbage.

read() returns the number of characters read, or -1 in case of error. It does NOT null-terminate its buffer, so using strlen() to find the amount of data read is wrong.

You shouldn't put a call to strlen() in the loop test condition, since you'll be re-evaluating it each iteration.

The casting of buf[i] to int in the printf statement is unnecessary. The extra arguments to variadic functions such as printf (that is, all of the arguments that make up the ...) undergo default argument promotion as follows:

• chars, shorts, and their unsigned counterparts are promoted to ints
• floats are promoted to doubles

Without a cast, buf[i] would get implicitly promoted to an int anyways, so adding the cast, while correct, makes the code more confusing to anyone reading it.

So, your code should look like this:

int fd = open("test.txt",O_RDONLY);
if(fd < 0)
{
    fprintf(stderr, "open failed: %s\n", strerror(errno));
    return;
}
char buf[128];
// It's better to use sizeof(buf) here, so we don't have to change it in case we
// change the size of buf.  -1 to leave space for the null terminator
int reader = read(fd,buf,sizeof(buf)-1);
if(reader < 0)
{
    fprintf(stderr, "read failed: %s\n", strerror(errno));
    close(fd);
    return;
}
buf[reader] = 0;  // add null terminator for safety
int i;
for (i=0; i < reader; i++)
{
    printf("%i: I read: %c", i, buf[i]);
}