Tag: Simple

Interface

I do not like too much the interface that you have defined for the function. I think that trying to combine reading integers and reading strings in one function makes it more difficult to use. For most C codes, I think the old UNIX guiding principle ("do one thing and do it right") provides excellent guidance. As such, I would probably have two distinct functions, one to read a string, the other to read a number.

I have not tested it to be sure, but at first glance, it also seems that if you ask to read a number, it will mean a poor failure in signaling. For example, if your file contains: foo=0 and you try to read foo as a number, it will set the number to 0 and return 0. But if the file contains foo=Joe instead, it seems to me that it's going to do exactly the same thing.

I would prefer that if that could not convert the rest of the line after the = to a number that it returns 1 to indicate that it has failed.

Memory allocation

Since you only use our left and right inside your function and release them again before coming back, it is probably not necessary to assign them to the job. You can just allocate them as local tables and use them. The obvious exception to this would be if you were targeting a tiny microcontroller that could have less than 256 bytes available on the stack. In this case, however, you will usually not have a file system, so the whole function is no longer relevant.

Standard library

I think of a judicious use of fscanf can simplify a little code.

To read a string, it seems that the obvious interface would simply be to return the value part of the pair, and if it is not found, return a NULL pointer:

char *read_string(FILE *file, char const *desired_name) { 
    char name(128);
    char val(128);

    while (fscanf(file, "%127(^=)=%127(^n)%*c", name, val) == 2) {
        if (0 == strcmp(name, desired_name)) {
            return strdup(val);
        }
    }
    return NULL;
}

When reading a number, we probably need something closer to your original interface, with a return value indicating success or failure and receiving a pointer on a int where we write the value (if found):

int read_int(FILE *file, char const *desired_name, int *ret) {
    char *temp = read_string(file, desired_name);

    char *stop;
    *ret = strtol(temp, &stop, 10);
    int ret_val = stop == NULL || *stop != '';
    free(temp);
    return ret_val;
}

Note that in the current state, the return value reflects the conversion of the all rest of the line (everything after =) to the desired number. If part of it can be converted, this part will be written to the destination, but the return will always indicate a failure. For example, if you had foo=1234abcd, trying to read foo as a number would give you 1234but returns 1 because the abcd some could not be converted.

It would also be possible to simply ignore the waste lying around, so foo=1234abcd would read 1234 part and return 0 to signal success.

This would simplify the code a bit:

int read_int(FILE *file, char const *desired_name, int *ret) {
    char *temp = read_string(file, desired_name);

    char *stop;
    *ret = strtol(temp, &stop, 10);
    free(temp);
    return stop == temp;
}

In this case, it is sufficient to check that at least one character has been converted, in which case stop has been advanced at least one character after the entry. Since we do not need to read the content of the strings to determine the return value, we no longer have to do the little dance with determining the value of return, then the release of the string and finally returning the return value.