Two variants of a db_create_resources(…) function After reading noobs, I'm interested in possible way to improve readability of the following short snippet. 


a struct pictdb_file is a small file system containing a metadata array and a file descriptor fpdb.
the function allocates the resources, and forwards initialization to db_create_logic.


How can I improve the readability of this snippet ? Please comment about: the syntax, white-lines, comments, function extracting, etc.

Also, which version do you prefer and why ?

version 1:

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    //allocate metadata
    db_file-&gt;metadata = calloc(db_file-&gt;header.max_files, sizeof(pict_metadata));

    if (db_file-&gt;metadata == NULL) {
        return ERR_OUT_OF_MEMORY;
    }

    //open fpdb
    db_file-&gt;fpdb = fopen(db_filename, "wb");

    if (db_file-&gt;fpdb == NULL) {
        free(db_file-&gt;metadata);
        db_file-&gt;metadata = NULL;
        return ERR_IO;
    }

    //forward
    enum error_codes e;
    e = db_create_logic(db_filename, db_file);

    //release resources
    free(db_file-&gt;metadata);
    db_file-&gt;metadata = NULL;
    db_close(db_file);

    return e;
}


version 2: (comment and blank-lines changes)

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    db_file-&gt;metadata = calloc(db_file-&gt;header.max_files, sizeof(pict_metadata));
    if (db_file-&gt;metadata == NULL) {
        return ERR_OUT_OF_MEMORY;
    }

    db_file-&gt;fpdb = fopen(db_filename, "wb");
    if (db_file-&gt;fpdb == NULL) {
        free(db_file-&gt;metadata);
        db_file-&gt;metadata = NULL;
        return ERR_IO;
    }

    enum error_codes e;
    e = db_create_logic(db_filename, db_file);

    free(db_file-&gt;metadata);
    db_file-&gt;metadata = NULL;
    db_close(db_file);

    return e;
}
There's no specification. What does this function do? What arguments should I pass? What are the preconditions on db_file? (It looks as if I might have to fill in some of the fields, but which ones?)
"resources" is spelled thus.
The function does not check the preconditions.
Mixing fopen and db_close seems confusing and error-prone. fopen should be paired with fclose, and db_close should be paired with db_open.
If the call to fopen fails, it looks as if you lose information about the cause of the error. fopen can fail due to permissions (EACCES), lack of disk space (ENOSPC), missing directory (ENOENT), and so on, but these all get converted to ERR_IO.
The function uses db_file-&gt;metadata to hold a pointer to some memory, but this memory is not actually used by the function, and by the end of the function the memory is freed and the pointer set to NULL. It seems wrong to use the db_file structure this way. If db_create_logic needs some temporary memory, it should allocate it itself (and preferably in a local variable, not in the db_file structure).
The cleanup logic for db_file-&gt;metadata appears twice. This is a bad idea: if you changed it in one of the places, would you remember to change it in the other? Consider refactoring the error handling code to look like this:

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    enum error_codes e;

    db_file-&gt;metadata = calloc(db_file-&gt;header.max_files,
                               sizeof *db_file-&gt;metadata);
    if (db_file-&gt;metadata == NULL) {
        e = ERR_OUT_OF_MEMORY;
        goto fail_calloc;
    }

    db_file-&gt;fpdb = fopen(db_filename, "wb");
    if (db_file-&gt;fpdb == NULL) {
        e = ERR_IO;
        goto fail_fopen;
    }

    e = db_create_logic(db_filename, db_file);
    db_close(db_file);
fail_fopen:
    free(db_file-&gt;metadata);
    db_file-&gt;metadata = NULL;
fail_calloc:
    return e;
}