Question

I wrote an application able to read in sqlite database thanks to this function:

struct query_res excucute_sql_statement(char *database, char *zSQL){
    sqlite3 *conn;
    sqlite3_stmt    *res;
    const char      *tail, *buf, *zErrMsg;
    struct query_res q_res;
    char table[MAXSTMTNUM][MAXCOLNUM][MAXSTRINGLEN];
    q_res.table = table;
    q_res.num = 0;
    int maxtry = 5, try = 0;

    while (sqlite3_open(database, &conn)) {
        if (try > maxtry)
            break;
        printf("Can not open database \'%s\'. %s\n", database, sqlite3_errmsg(conn));
        usleep(50000);
        try ++;
    }

    if (sqlite3_exec(conn, zSQL, callback, &q_res, &zErrMsg)){
        printf("Excecuting %s\n", zSQL);
        printf("We did not get any data! error %s\n",zErrMsg);
        if(sqlite3_finalize(conn))
            printf("Can not finalize database. %s\n", sqlite3_errmsg(conn));
        if(sqlite3_close(conn))
            printf("Can not close database. %s\n", sqlite3_errmsg(conn));
        return q_res;
    }

    sqlite3_free(zSQL);

    if(sqlite3_close(conn))
        printf("Can not close database. %s\n", sqlite3_errmsg(conn));

    return q_res;
}

For each line returned the function callback is called:

static int callback(void *buf, int argc, char **argv, char **azColName){
    int i;
    struct query_res *q_res;
    q_res = (struct query_res *)buf;
    if (q_res->num >= MAXSTMTNUM)
        return 0;

    q_res->table[q_res->num] = calloc(argc, sizeof(char *));

    for(i=0; i<argc; i++){
        if (i >= MAXCOLNUM)
            break;  
        q_res->table[q_res->num][i] = calloc(((strlen(argv[i]) < MAXSTRINGLEN) ? strlen(argv[i]) : MAXSTRINGLEN), sizeof(char));
        strncpy(q_res->table[q_res->num][i], argv[i], ((strlen(argv[i]) < MAXSTRINGLEN) ? strlen(argv[i]) : MAXSTRINGLEN));
    }
    q_res->num ++;

    return 0;
}

Here is the extract of the code where excucute_sql_statement is called:

struct query_res    res;
res = excucute_sql_statement(database, zSQL);


directions = malloc(sizeof (struct direction_list));
directions->directions = calloc(5,  sizeof (struct direction));

double cur_dist, min_dist = 30;
float s_lat, s_lon, e_lat, e_lon;
directions->direction_num = 0;

//printf("Res table num %d\n", res.num);

//printf("First elem %s\n", res.table[0][0]);

for (i = 0 ; i < res.num ; i++){
    //printf("%d. %s|%s|%s|%s|%s|%s\n", i, res.table[i][0], res.table[i][1], res.table[i][2], res.table[i][3], res.table[i][4], res.table[i][5]);
    sscanf(res.table[i][1], "%g", &s_lat);
    sscanf(res.table[i][2], "%g", &s_lon);
    sscanf(res.table[i][3], "%g", &e_lat);
    sscanf(res.table[i][4], "%g", &e_lon);
    sscanf(res.table[i][0], "%d", &rs);
    sscanf(res.table[i][5], "%d", &rp);
    //printf("New seg start: %g,%g end %g,%g rs %d rp %d\n", s_lat, s_lon, e_lat, e_lon, rs, rp);
    cur_dist = (gps_distance(location.lat, location.lon, s_lat, s_lon)
        + gps_distance(location.lat, location.lon, e_lat, e_lon)) / 2;
    //printf("Current direction num %d \n", directions->direction_num);
    //printf("cur_dist %f\n", cur_dist);
    if (cur_dist < min_dist){   
        directions->directions[0] = fill_direction(rs, rp, database);
        directions->direction_num = 1;
        min_dist = cur_dist;
    }
    else if (cur_dist == min_dist){
        directions->directions[directions->direction_num] = fill_direction(rs, rp, database);
        directions->direction_num ++;
    }
}

These functions works fine and give the expected result but when running valgrind, I have the following output:

==22808== Thread 2:
==22808== Invalid read of size 4
==22808==    at 0x804946B: get_all_possible_directions (util.c:240)
==22808==    by 0x8049D73: start_direction_detection (direction_detection.c:293)
==22808==    by 0x40C41C88: ???
==22808==  Address 0x4f03690 is not stack'd, malloc'd or (recently) free'd
==22808== 
==22808== Invalid read of size 1
==22808==    at 0x402F5C3: __GI___rawmemchr (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==22808==    by 0x40C246E: _IO_str_init_static_internal (strops.c:44)
==22808==    by 0x8049D73: start_direction_detection (direction_detection.c:293)
==22808==    by 0x40C41C88: ???
==22808==  Address 0x45aa01b is 0 bytes after a block of size 11 alloc'd
==22808==    at 0x402B965: calloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==22808==    by 0x8048F59: callback (util.c:57)
==22808== 
==22808== Invalid read of size 4
==22808==    at 0x8049487: get_all_possible_directions (util.c:241)
==22808==    by 0x8049D73: start_direction_detection (direction_detection.c:293)
==22808==    by 0x40C41C88: ???
==22808==  Address 0x4f03690 is not stack'd, malloc'd or (recently) free'd

and so on ...

Note that line 240 corresponds to the first scanf statement.

I think there is something wrong with my table initialization. Maybe here:

q_res->table[q_res->num] = calloc(argc, sizeof(char *));

Do you have any idea why valgrind is triggering this error?

Thanks


[update from comment:]

struct query_res is composed of char ***table and int num.

Was it helpful?

Solution

In excucute_sql_statement(), here

q_res.table = table;

you are copying a reference to stack-local storage (table) into the stucture returned by the function.

Stack-local storage becomes invalid as soon as the function returns, so the structure's member table refers to invalid (unallocated) memory after the function has returned.

To fix this modifiy excucute_sql_statement()

struct query_res excucute_sql_statement(char *database, char *zSQL)
{
  [...]

  /* char table[MAXSTMTNUM][MAXCOLNUM][MAXSTRINGLEN]; */ /* Delete this line. */
  q_res.table = NULL;
  q_res.num = 0;

and callback()

static int callback(void *buf, int argc, char **argv, char **azColName)
{
  size_t i;
  struct query_res * q_res = (struct query_res *) buf;

  /* Resize statement table, adding one new entry. */
  q_res->table = realloc(q_res->table, (q_res->num + 1) * sizeof(*q_res->table));

  /* Allocate new argument table. */
  /* (Allocate +1 for a stopper element which stays NULL to be able to detect the end of the table.) */
  q_res->table[q_res->num] = calloc(argc + 1, sizeof(*q_res->table[q_res->num]));

  for(i=0; i<argc; ++i)
  {
    /* Allocate entry for argument, that is characters for argument. */
    q_res->table[q_res->num][i] = malloc(strlen(argv[i]) + 1);
    /* Copy argument. */
    strcpy(q_res->table[q_res->num][i], argv[i]);
  }

  q_res->num++;

  return 0;
}

(untested)

Also please note that the whole code (yours and mine) is missing proper error checking. Here especially the retuned values of the allocating calls to malooc/calloc/reallocshall be tested against NULL!

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top