Looking for improvement ideas

Question

Hello,

gcc 4.4.4 c89

Is there a better way to do this?

I have the following code to read in from a text file. The text file contains lines like this:

4 5 6 1 5 7 5 9 5 3 5 7 8 3 2 3 
7 2 3 4 5 3 7 9 3 2 5 6

I have given only 2 lines example, but there could be more and each with different lengths.

What I need to do is get the numbers into a buffer so that I can analsys them. Which is easy enough.

However, I am looking for a solution that won't overwrite the buffer for each line. So my result buffer should contain the following:

4 5 6 1 5 7 5 9 5 3 5 7 8 3 2 3 7 2 3 4 5 3 7 9 3 2 5 6

So I am using fgets to read in the lines, and passing that line to my analyse function.

However, I need the increment value in the for loop to start where the last one finished.

I have set the device_buff to static. Is that safe. I am not keen to use static variable inside a function, as they are not thread safe and constitutes to a global variable.

int g_load_devices_numbers(void)
{
    fget(line_read, DEVICE_SIZE, fp) == NULL) {
        analyse_device_numbers(line_read);   
    }
}

static void analyse_device_numbers(const char * const device_line)
{
    size_t i = 0;
    static char device_buff[1024] = {0};
    static size_t device_counter = 0;
    /* Start inserting the last index */
    static size_t buff_counter = 0;

    /* copy each number into the char array
     * only copy up to the 'return' as fgets always inserts one */
    for(i = 0; device_line[i] != '\n'; i++, buff_counter++) {
        device_buff[buff_counter] = device_line[i];
        /* Only count numbers and not spaces */
        if(isspace(device_buff[buff_counter]) == 0) {
            device_counter++;
        }
    }

    /* nul terminate the vote buffer */
    device_buff[buff_counter] = '\0';

}

Many thanks for any suggestions,

Answer 1

No, using a static buffer for device_buff is not safe here. Not because of the buffer itself, but because it has a limited size (1024 items) that is not checked.

It would be safer to provide as input parameters to analyse_device_numbers the buffer where data must be stored and the length of that buffer. Length must still be checked, to avoid writing after the last cell of the provided buffer and you have to choose some error management convention (like returning -1 from analyse_device_numbers when a buffer overflow occured).

To always write in the same target buffer the usual trick if to move the beginning of the provided buffer (to take into account items already stored) and to reduce the total length by the same amout. This can be done in the external loop that call g_load_device_numbers.