Reading the different columns of csv file in C

Question

I have a csv file which has 2 columns named; timestamp, SNR (int values). I have to write a function which asks first the user input; which value user wants? sample file:

 timestamp        ;       SNR   
 16:15:12:468     ;       15
 16:15:12:968     ;       20

For Example: If I enter SNR, the function should give me the column no. of SNR; (that is column 2 here) as well as the values of SNR.

Output : Col. no. is 2         
            15       /* time difference of ((16:15:12:968)-(16:15:12:458) = 500ms between these two output values*/  
            20

But these values should be given as output on a certain time interval. This implies that the timestamp column has to be read first and the difference between the two timestamp (current & next) values should be calculated. Now the SNR should be given as output on the interval of difference between these two timestamp values. I do not want to use array or structures because I don’t want to store values; I just require these values to pass on to other application on certain time interval.

I wrote the following code. I could get the user input and output the column no. of file, but I am not able to get the content of these columns. I used the switch case in my program, but I am not getting why this switch case is not working. I have also written a function to get the time difference between these two timestamps, but I am not getting how to combine it in this function.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <windows.h>
#include <time.h>
#define BUFFER_SIZE 1024
#define num_rows 100

const char* gettime(char* line, int num )
{
    const char* tok;
    for (tok = strtok(line, ";");tok && *tok;tok = strtok(NULL, ";\n"))
    {
        if (!num--)
        //Sleep(500);
        return tok;
    }
    return NULL;
}
const char* getSNR(char* line, int num )
{
    const char* tok;
    for (tok = strtok(line, ";");tok && *tok;tok = strtok(NULL, ";\n"))
    {
        if (!num--)
        //Sleep(500);
        return atoi(tok);
    }
    return NULL;
}

struct tm mytime;


int main ()
{
    int hh, mm;
    float ss, ms;
    mytime.tm_year = 2015 - 1900;               /* To initialize the struct tm*/
    mytime.tm_mon = 6;
    mytime.tm_mday = 15;
    int count;
    int value;
    char text[25];
    char *buffer;
    FILE *fp;
    char *token;
    char *tok;
    char line [100];
    char time_buffer[100];
    char **timestamp;                               /*Dynamic allocated array*/
    unsigned long ul_second_prev ;
    unsigned long ul_second_current;
    int i=0, j=0, k=1;
    int ui_time_diff, ui_SNR;
    time_t time_prev, time_current;                 /*Dynamic allocated array*/
    int timediff ;
    timestamp = malloc(num_rows*sizeof(char*));


if ((timestamp)== NULL)
    {
        printf("Error: out of memory");
    }

if ((fp=fopen("testfile.csv", "r"))==NULL)
{
    printf ("file cannot be opened");
    return 1;
}


buffer = malloc (BUFFER_SIZE); /*Allocate memory in buffer to read the file*/

if (buffer == NULL)
{
    printf("Error: Out of Memory");
    return 1;
}


    fgets(line, BUFFER_SIZE, fp);
    printf ("%s", line);
    printf ("enter your input\n");
    scanf("%s" , &text);
for (tok = strtok(line, ";");tok && *tok;tok = strtok(NULL, ";\n"))
{
        value = strcmp (tok, text);
        if(value ==0)
            printf("col. no. is %d", k);
        else k++ ;
}

while (fgets(line, BUFFER_SIZE, fp))
{

    char* tmp = strdup(line);

switch (k)
{
    case 1:
        gettime(tmp, 1);
        printf ( "%s",tok );
        break;
    case 2:
        getSNR(tmp, 2);
        printf ( "%s",tok );
        break;
}
free(tmp);
}

Answer 1

Ok apart from the myriads of unused variables and at least one unused include (<windows.h>), gcc gives also these warnings that probably point out your problems:

> gcc -std=c99 -Wall -Wextra -pedantic -oq q.c
q.c: In function ‘getSNR’:
q.c:26:9: warning: return makes pointer from integer without a cast
         return atoi(tok);
         ^
q.c: In function ‘main’:
q.c:84:5: warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘char (*)[25]’ [-Wformat=]
     scanf("%s" , &text);
     ^
q.c:96:5: warning: implicit declaration of function ‘strdup’ [-Wimplicit-function-declaration]
     char* tmp = strdup(line);
     ^
q.c:96:17: warning: initialization makes pointer from integer without a cast
     char* tmp = strdup(line);
                 ^
q.c:110:1: error: expected declaration or statement at end of input
 }
 ^

Your getSNR() returns an integer, but is declared to return a character pointer (aka: string). You discard the return value and just feed an (uninitialized!) char *tok (which should be const to take the return value) to the %s format specifier.

In your scanf, apart from using the wrong pointer type (that's what the warning is about), you have a buffer overflow. Welcome crackers. NEVER EVER use scanf("%s", ...), there's no way to tell how much data it will read. Use fgets(text, 25, stdin) in that case here.

You should learn two things when programming C: First, be accurate and precise, not erratic. Second, let your compiler warn about everything (using the appropriate switches if necessary). A compiler warning means in 98% of the cases a programming error or bug.

That being said, it's silly to have 2 nearly identical functions. Do something like this:

const char* getfield(char* line, int num )
{
    const char* tok;
    for (tok = strtok(line, ";");tok && *tok;tok = strtok(NULL, ";\n"))
    {
        if (!num--)
        return tok;
    }
    return NULL;
}

Still a bit strange this loop, but that's a question of style ... and then later (note that you actually assign the value returned from your function):

while (fgets(line, BUFFER_SIZE, fp))
{
    switch (k)
    {
        case 1:
            tok = getfield(line, 1);
            printf ( "%s",tok );
            break;
        case 2:
            tok = getfield(line, 2);
            printf ( "%d",atoi(tok) );
            break;
    }
}

Also note the allocated tmp you used doesn't make any sense, as your line buffer gets overwritten in the next iteration of the loop anyways. So why take a copy?