# Reading a text file that may have embedded nulls

I've always used fgets to read a file. However, I want to read a file that may have embedded \0. I thought of using ftell to query the size, but it doesn't seem to work on all files.

I've got a test file,

31 32 33 00 34 35 36 0A E2 82 AC 0D 0A 61


Here is my fgets.

#include <stdlib.h> /* EXIT */
#include <stdio.h>  /* printf perror fputc fread */
#include <string.h> /* strlen */
#include <errno.h>  /* errno */
#include <assert.h> /* assert */

int main(void) {
char file[1000], *f = file, *a;
const int granularity = 80;
int is_done = 0;

for( ; ; ) {
/* Fail when contents bigger than the size;
would be a good spot to use realloc. */
if(granularity > sizeof file - (f - file))
{ errno = ERANGE; break; }
if(!fgets(f, granularity, stdin))
{ if(ferror(stdin)) break; is_done = 1; break; }
f += strlen(f);
}

for(a = file; a < f; a++) printf("%02hhX ", *a);
fputc('\n', stdout);
return is_done ? EXIT_SUCCESS : (perror("stdin"), EXIT_FAILURE);
}


Running this, (I'm on a UNIX-like machine,)

$bin/fgets < test 31 32 33 E2 82 AC 0D 0A 61  Here is my fread. #include <stdlib.h> /* EXIT */ #include <stdio.h> /* printf perror fputc fread */ #include <errno.h> /* errno */ #include <assert.h> /* assert */ int main(void) { char file[1000], *f = file, *a; const int granularity = 80; size_t read; int is_done = 0; for( ; ; ) { if(granularity > sizeof file - (f - file)) { errno = ERANGE; break; } read = fread(f, 1, granularity, stdin); if(ferror(stdin)) break; assert(read >= 0 && read <= granularity); f += read; if(read != granularity) { is_done = 1; break; } } for(a = file; a < f; a++) printf("%02hhX ", *a); fputc('\n', stdout); return is_done ? EXIT_SUCCESS : (perror("stdin"), EXIT_FAILURE); }  Running this, $ bin/fread < test
31 32 33 00 34 35 36 0A E2 82 AC 0D 0A 61


I would like to know if this is pedantically correct and how I improve.

• If a file contains NULs, then it would usually be called a binary file rather than a text file. Apr 27 '19 at 1:53
• I thought that depended on what encoding it's in. Would UTF-8 be a potentially binary file, then? en.wikipedia.org/wiki/UTF-8#Modified_UTF-8 Is there an unambiguous definition of text/binary?
– Neil
Apr 27 '19 at 18:18
• UTF-8 does not have NUL bytes. UTF-16 would. Apr 27 '19 at 18:20
• What is the point of granularity vs. just using fread(fine, 1, sizeof file, stdin)? Apr 27 '19 at 20:09
• @200_success Although uncommon to have a null character in a text file, C's read_file does not exclude them and robust code does have have UB when reading them. Neil's goal is a good one. Apr 27 '19 at 20:21

• the variable file is not really a file but a buffer
• the variable f (which usually stands for file) is a pointer into that buffer
• the variable a has a name which does not convey any meaning at all
• if the first if statement you indent the brace in the next line, and in the very last if statement the body is in the same line
• there are spaces missing in the code at places where I expect them, such as after an if or for
• the forever loop is usually written as for (;;), not for( ; ; )
• the main block of the code is inside the for loop, and there's not a single empty line in that part. This suggests that the whole block is doing a single thing with no possible interruption or logical break
• the comma operator is generally frowned upon
• is_done is not really about the work being done, it's more about being successful

Because of all the above reasons, I would write the code differently:

#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>

int main(void) {
char buf[1000];
size_t buflen = 0;
const size_t granularity = 80;

while (true) {
if (granularity > sizeof buf - buflen) {
errno = ERANGE;
break;
}

if (ferror(stdin))
break;

}

for (size_t i = 0; i < buflen; i++)
printf("%02hhX ", buf[i]);
fputc('\n', stdout);

if (ferror(stdin)) {
perror("stdin");
return EXIT_FAILURE;
}
}


And here is what I changed:

• I renamed all variables to match their purpose
• I replaced the various pointers into the buffer with indexes
• I removed the is_done variable since the program should not print an error just because the file is a multiple of 80 bytes
• I moved the variable a to a smaller scope by declaring it inside the for loop where it is used; I also renamed it to i, since it is now an index instead of a pointer
• I replaced the comma operator with an if statement, since that is the form that is commonly used
• I included <stdbool.h> to have a boolean type and the constants true and false
• I replaced for (;;) with while (true), which is less magic
• I sorted the included headers alphabetically since for headers from the C standard library, the order doesn't matter
• I renamed the variable read to nread, to avoid possible conflicts with the POSIX function of the same name
• This is an excellent review. ssize_t and bool are C99, but agree. I'm more interested in is how your control flow differs from mine; pubs.opengroup.org/onlinepubs/007904875/functions/fread.html says nothing about returning a negative value, though in practice that's what it does. Could you elaborate?
– Neil
Apr 27 '19 at 17:23
• Oops, I must have remembered something wrong then. Sorry. I fixed my review. Thanks for notifying me. Apr 27 '19 at 17:28
• No, I thought so, too; in practice a negative value indicates error, but upon reading the spec carefully, I see no indication, unless it's in one of the links. Usually I use fgets. If I'm understanding this, you could have a partial read and errno would be set, it would return a positive value.
– Neil
Apr 27 '19 at 18:10
• @NeilEdelman "ssize_t and bool are C99" --> ssize_t is not C99. it is not part of any C standard, yet common *nix. Apr 27 '19 at 19:58
• The only part I would disagree with is replacing for(;;) (read: forever), as that's very idiomatic. Apr 28 '19 at 15:22

Roland Illig provided an excellent review; there are a couple of points I'd like to add:

The standard header file <stdio.h> defines the macro/constant BUFSIZ. This macro was developed primarily for input and output buffers. In the original C it was defined as 1024, but now it varies from system to system, probably based on the file system blocking size.

It might have been better to define to define the character array using BUFSIZ.

The assert() macro is generally useful as a debugging tool and not included in production code. It will be optimized out of the code if the macro NDEBUG is defined.

The first time I read through the code, I missed all of the break; statements; it might be better if each statement is on a separate line.

• In your opinion, is not ferror an absolute guarantee that the return value is between [0, read]?
– Neil
Apr 28 '19 at 21:24

Reading a text file that may have embedded nulls

fgets() is simply not the best tool to to do this.

Code that well handles text files with null characters employs *nix getline() or similar functions.

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

int main(void) {
size_t sz = 0;
char *buf = NULL;
unsigned long long line = 0;
ssize_t count;
char ch = '\n';
while ((count = getline(&buf, &sz, stdin)) > 0) {
printf("%llu", ++line);
for (ssize_t i = 0; i < count; i++) {
ch = buf[i];
printf(" %02hhX:", ch);
}
}
if (ch != '\n') {
printf("\n");
}
free(buf);
if (ferror(stdin)) {
perror("stdin");
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
}

• If you're using GNU C99, I think this is the way to go.
– Neil
Apr 27 '19 at 21:42

I would normally use ftell and allocate the buffer size needed, but I assume that you have some non-standard Unix Distro with specific environment or compiler settings, if you say that ftell doesnt work sometimes.

I provide an example with fix buffer size, just for demonstration. Buffer size reallocation and more error checking would probably be needed for production code.

Regarding code, generally, I always preferr staying generic and grouping code in functions. It's not only making code easier to read, but also makes a thinking process easier. And there's a reusabillity point too, of course.

So this would be my refactoring - suggestion (I didn't pay much attention to your error codes. And it could be that you have to put declarations on top, if you are using some older C compiler like C89 etc.):

#include <stdio.h>

#define GRANULARITY 80
#define MIN(a, b) (((a) < (b)) ? (a) : (b))

size_t read_file(FILE* fd,      // file descriptor
char *buf, // buffer
size_t size); // buffer size

int main(int argc, char* argv[])
{
char buf[1000];

{
for (int i = 0; i < sizeof(buf); ++i)
printf("%02hhX ", buf[i]);
}
else if (errno == ERANGE)
{
// error - there are more bytes to read, handle it
}
else if (ferror(fd))
{
// stream error - handle it
}

return 0;
}

char *buf,
size_t size)
{
char *pos = buf;
size_t n = 0;

while ( (n = fread(pos, 1, MIN(GRANULARITY, buf+size-pos), fd)) > 0 )
{
pos += n;
}

if (!feof(fd))
errno = ERANGE;   // buf too small - there are more bytes to read


• That's a good generalisation. If you set errno to eg, ERANGE, and always returned -1, then the code would have one less level of errors to check for. My intention was to write it to work on any compliant C90 system, so I've been careful what assumptions I'm making, viz c-faq.com/osdep/filesize.html and c-faq.com/stdio/textvsbinary.html.
• When reading large files that can fit in size_t, num_read += n; can readily exceed ssize_t range. Recommend staying with size_t and not convolute with ssize_t. Apr 27 '19 at 20:13
• Pedantically if (ferror(fd)) can be true even with no read errors here, as ferror() can be true before read_file() is called. Apr 27 '19 at 20:15