Skip to main content
added 2 characters in body
Source Link
Reinderien
  • 65.3k
  • 5
  • 69
  • 187
/*
** Read the files contents and return them.
*/
char* read_file(const char* fname) {

    char* buffer = 0;
    long length;

    FILE* fp = fopen(fname, "rb");

    if (fp) {
        fseek(fp, 0, SEEK_END);
        length = ftell(fp);
        fseek(fp, 0, SEEK_SET);
        buffer = (char*)malloc((length + 1) * sizeof(char));
        if (buffer) {
            fread(buffer, sizeof(char), length, fp);
        }
        fclose(fp);
    }
    buffer[length] = '\0';
    return buffer;
}
** Read the files contents and return them.
*/
char* read_file(const char* fname) {

    char* buffer = 0;
    long length;

    FILE* fp = fopen(fname, "rb");

    if (fp) {
        fseek(fp, 0, SEEK_END);
        length = ftell(fp);
        fseek(fp, 0, SEEK_SET);
        buffer = (char*)malloc((length + 1) * sizeof(char));
        if (buffer) {
            fread(buffer, sizeof(char), length, fp);
        }
        fclose(fp);
    }
    buffer[length] = '\0';
    return buffer;
}
/*
** Read the files contents and return them.
*/
char* read_file(const char* fname) {

    char* buffer = 0;
    long length;

    FILE* fp = fopen(fname, "rb");

    if (fp) {
        fseek(fp, 0, SEEK_END);
        length = ftell(fp);
        fseek(fp, 0, SEEK_SET);
        buffer = (char*)malloc((length + 1) * sizeof(char));
        if (buffer) {
            fread(buffer, sizeof(char), length, fp);
        }
        fclose(fp);
    }
    buffer[length] = '\0';
    return buffer;
}
Spelling and grammar
Source Link
Toby Speight
  • 81.7k
  • 14
  • 101
  • 308

Since the code is currently all in one file, all of the functions could be declared as static functions,functions; that won't be true if the code gets broken up.

In the above function the statement buffer[length] = '\0'; has undefineundefined behavior if the malloc for bufferallocation of buffer failed, so that assignment should be within the if (buffer) { statement. If the code used calloc()calloc() rather than malloc(), that assignment is unnecessary since buffer would be initialized to all zeros.

There is no need to cast the mallocresult of malloc() to char *, since malloc returns void *. In the older version of C it was only necessary to cast the return from malloc to another type if the type wasn't char *char * since the older versions of malloc returned char *.

When using malloc()malloc(), calloc()calloc() or realloc()realloc() in C a common convention is to sizeof(*PTR)use sizeof *PTR rather sizeof(PTR_TYPE)sizeof (PTR_TYPE), as this make the code easier to maintain and less error prone, since less editing is required if the type of the pointer changes.

The function str_replace() has some variable names that need to be clarified,: the variable name src is pretty common, but trg and with are not so clear, I can guess that trg is target, but it isn't really clear. It really isn't clear what the variable ins is.

The declaration for retVal isn't necessary until the call to malloc() in the str_replace() function. The variable ins is only necessary ionin the whilewhile loop and so can be declared within the while loop.

Since the code is currently all in one file all of the functions could be declared as static functions, that won't be true if the code gets broken up.

In the above function the statement buffer[length] = '\0'; has undefine behavior if the malloc for buffer failed, so that assignment should be within the if (buffer) { statement. If the code used calloc() rather than malloc(), that assignment is unnecessary since buffer would be initialized to all zeros.

There is no need to cast the malloc to char *, since malloc returns void *. In the older version of C it was only necessary to cast the return from malloc to another type if the type wasn't char * since the older versions of malloc returned char *.

When using malloc(), calloc() or realloc() in C a common convention is to sizeof(*PTR) rather sizeof(PTR_TYPE), this make the code easier to maintain and less error prone, since less editing is required if the type of the pointer changes.

The function str_replace() has some variable names that need to be clarified, the variable name src is pretty common, but trg and with are not so clear, I can guess that trg is target, but it isn't really clear. It really isn't clear what the variable ins is.

The declaration for retVal isn't necessary until the call to malloc() in the str_replace() function. The variable ins is only necessary ion the while loop and can be declared within the while loop.

Since the code is currently all in one file, all of the functions could be declared as static functions; that won't be true if the code gets broken up.

In the above function the statement buffer[length] = '\0'; has undefined behavior if the allocation of buffer failed, so that assignment should be within the if (buffer) { statement. If the code used calloc() rather than malloc(), that assignment is unnecessary since buffer would be initialized to all zeros.

There is no need to cast the result of malloc() to char *, since malloc returns void *. In the older version of C it was only necessary to cast the return from malloc to another type if the type wasn't char * since the older versions of malloc returned char *.

When using malloc(), calloc() or realloc() in C a common convention is to use sizeof *PTR rather sizeof (PTR_TYPE), as this make the code easier to maintain and less error prone, since less editing is required if the type of the pointer changes.

The function str_replace() has some variable names that need to be clarified: the variable name src is pretty common, but trg and with are not so clear, I can guess that trg is target, but it isn't really clear. It really isn't clear what the variable ins is.

The declaration for retVal isn't necessary until the call to malloc() in the str_replace() function. The variable ins is only necessary in the while loop and so can be declared within the loop.

Source Link
pacmaninbw
  • 24.8k
  • 11
  • 46
  • 109

General Observations

Does my code meet C99 standards?

Not exactly, it meets an earlier standard than C99, since C99 is generally backwards compatible with the earlier standards it compiles just fine in C99.

An example of features in C99 that aren't used is that for loop control variables can be declared in the for statement, so that

static int callback(void *ignore, int argc, char **argv, char **azColName) {
    int i;
    for (i=0; i<argc; i++) {
        printf("%s = %s\n", azColName[i], argv[i] ? argv[1] : "NULL");
    }
    printf("\n");
    return 0;
}

can be written this way:

static int callback(void* ignore, int argc, char** argv, char** azColName) {
    for (int i = 0; i < argc; i++) {
        printf("%s = %s\n", azColName[i], argv[i] ? argv[1] : "NULL");
    }
    printf("\n");
    return 0;
}

The code is generally portable.

Rather than hard coding the name of the database in the execute_command() function, the code would be more useful if the program parameters accepted the name of the database as well as the name of the input file.

Program Organization

The code is complex enough that you could break it up into multiple source files with header files. This would allow the main program to run with different database solutions by using different header and source files. It would also make the code easier to maintain.

Since the code is currently all in one file all of the functions could be declared as static functions, that won't be true if the code gets broken up.

Remove Code That Isn't Used

The following symbolic constants are not used in the program:

// FILE constants for opening and reading files.
#define FILE_OK 0
#define FILE_NOT_EXIST 1
#define FILE_TOO_LARGE 2
#define FILE_READ_ERROR 3

It would be better to remove them from the code.

Declare the Variables as Needed

In the original version of C back in the 1970s and 1980s variables had to be declared at the top of the function. That is no longer the case, and a recommended programming practice to declare the variable as needed. In C the language doesn't provide a default initialization of the variable so variables should be initialized as part of the declaration. For readability and maintainability each variable should be declared and initialized on its own line. This applies to the for loop I mentioned above as well as in other functions.

In the following function the variable length can be declared within if (fp) {

** Read the files contents and return them.
*/
char* read_file(const char* fname) {

    char* buffer = 0;
    long length;

    FILE* fp = fopen(fname, "rb");

    if (fp) {
        fseek(fp, 0, SEEK_END);
        length = ftell(fp);
        fseek(fp, 0, SEEK_SET);
        buffer = (char*)malloc((length + 1) * sizeof(char));
        if (buffer) {
            fread(buffer, sizeof(char), length, fp);
        }
        fclose(fp);
    }
    buffer[length] = '\0';
    return buffer;
}

In the above function the statement buffer[length] = '\0'; has undefine behavior if the malloc for buffer failed, so that assignment should be within the if (buffer) { statement. If the code used calloc() rather than malloc(), that assignment is unnecessary since buffer would be initialized to all zeros.

There is no need to cast the malloc to char *, since malloc returns void *. In the older version of C it was only necessary to cast the return from malloc to another type if the type wasn't char * since the older versions of malloc returned char *.

Convention When Using Memory Allocation in C

When using malloc(), calloc() or realloc() in C a common convention is to sizeof(*PTR) rather sizeof(PTR_TYPE), this make the code easier to maintain and less error prone, since less editing is required if the type of the pointer changes.

Variable Names

The function str_replace() has some variable names that need to be clarified, the variable name src is pretty common, but trg and with are not so clear, I can guess that trg is target, but it isn't really clear. It really isn't clear what the variable ins is.

The declaration for retVal isn't necessary until the call to malloc() in the str_replace() function. The variable ins is only necessary ion the while loop and can be declared within the while loop.