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.