Reading separate components of 8.3 filename into proper filename in C

I need to read an archive file format which stores filenames as 8 characters for the name (null-terminated if less than 8 characters, not otherwise), immediately followed by 3 characters for the extension (i.e. without a dot). So basically, I need to read the components and add a dot between the name and extension (name.ext).

Is this the way to go, or is there a better/more common way of doing it?

#include   // (f)printf, fopen, fseek, ftell, fread, fclose
#include  // calloc, free

void read_filename(FILE *arc, char *filename)
{
    // read file name
    if(8 != fread(&filename[0], 1, 8, arc))
    {
        fprintf(stderr, "ERROR: Failed reading file name at 0x%08X\n", ftell(arc));
        exit(EXIT_FAILURE);
    }

    // find the "real" end of the name and append a dot
    int c;
    for(c=0; c < 8 && filename[c] != '\0'; c++);
    filename[c] = '.';

    // read extension into the offset of the dot + 1
    if(3 != fread(&filename[c+1], 1, 3, arc))
    {
        fprintf(stderr, "ERROR: Failed reading file extension at 0x%08X\n", ftell(arc));
        exit(EXIT_FAILURE);
    }
}

int main(int argc, char *argv[])
{
    if (argc != 2) return 1;

    // open the archive
    FILE *arc = fopen(argv[1], "rb");
    if(arc==NULL)
    {
        fprintf(stderr, "ERROR: Could not open file.\n");
        exit(EXIT_FAILURE);
    }

    // seek to 0x28, which is the offset of the first filename
    fseek(arc, 0x28, SEEK_SET);

    // allocate cleared memory for the file name
    char *filename = (char *)calloc(12, 1);

    // read the file name into the memory block allocated
    read_filename(arc, filename);

    printf("First filename: %s...", filename);

    free(filename);
    fclose(arc);

    return 0;
}

An example of a filename might look like this (hex dump):

0x00000028   46 49 4C 45 4E 41 4D 00 65 78 74 00   FILENAM ext

the space in the example above is a null byte.

or it can look like this:

```
0x00000028 46 49 4C 45 4E 41 4D 45 65 78 74 00 F

Here are some things that may help you improve your code.

Avoid "tricky" loops

There is nothing technically wrong with this line:

for(c=0; c < 8 && filename[c] != '\0'; c++);

However, because one doesn't usually write a for loop with no body, it would probably be useful for your code to call attention to the fact that there's nothing happening within the loop. You could write it like this:

for(c=0; c < 8 && filename[c] != '\0'; c++) {
    // do nothing 
}

Or another way:

for(c=0; c < 8 && filename[c] != '\0'; c++)
    continue;

All will likely compile to the exact same object code.

Don't mix I/O with data operations

Generally speaking having a printf within a data manipulation function such as read_filename is not a good idea. It makes it harder to reuse the function. It's OK for diagnostics and for troubleshooting, but it would be better to omit it from the final version. Similarly, it would be better if the fread would read the entire 8.3 filename into a memory buffer and then pass the memory buffer to code that would do the parsing. That cleanly separates I/O from data manipulation and is probably more efficient as well.

Return an error code

The read_filename function can detect errors, but summarily exits the entire program rather than returning an error code that would allow the calling function to decide how to handle the error. A more robust design that might allow the code to be reused would return an error code.

Eliminate "magic numbers"

Things like if (argc != 2) are OK, because programmers reading this will know what that code is doing. However, when you write if(8 != fread(&filename[0], 1, 8, arc)) it's not at all obvious whether 8 is the correct value, or exactly what it represents. Better would be to use a named const value there such as const size_t FILENAMESIZE = 8;

Be careful with assumptions about argument size

In the fprintf functions, the parameter should be 0x%08lX rather than 0x%08X because ftell() returns a long.

Try this

Here's one way to do this, incorporating all of these suggestions:

// max size of 8.3 filename
const size_t FILENAMELEN = 11u;
const size_t EXTOFFSET = 8u;
const size_t EXTLEN = 3u;

char *read_filename(FILE *arc)
{
    char *filename = malloc(FILENAMELEN+2);
    if (filename == NULL) {
        return filename;
    }
    filename[FILENAMELEN] = '\0';
    if (FILENAMELEN != fread(filename, 1u, FILENAMELEN, arc)) {
        free(filename);
        return NULL;
    }
    return filename;
}

char *parse_filename(char *filename)
{
    if (filename == NULL) {
        return filename;
    }
    // insert the '.'
    memmove(&filename[EXTOFFSET+1], &filename[EXTOFFSET], EXTLEN);
    filename[EXTOFFSET] = '.';
    size_t i;
    for (i=0; filename[i] && i < EXTOFFSET; ++i) {
    }
    if (i < EXTOFFSET) {
        memmove(&filename[i], &filename[EXTOFFSET], EXTLEN+2);
    }
    return filename;
}

StackExchange Code Review Q#77939, answer score: 4