[sqlite] Quick Start Example Potential Memory Leak: zErrMsg

[Mike Chirico]

I believe the following quick start example leaves
the program open to a potential memory leak when
the call to sqlite3_exec is not equal to SQLITE_OK 
because zErrMsg is not explicitly freed.

Reference the example:

   http://www.sqlite.org/quickstart.html

The error can be reproduced by compiling the example from the
above link and running it for the following SQL statement:

  $ gcc quickstart.c  -Wall -W -O2 -Wl,-R/usr/local/lib \
            -lsqlite3


Now execute the following statement.

  $ ./a.out tmptable "drop table junk"

Since the table "junk" does not exist, zErrMsg gets memory allocated
to it (legacy.c:129) in the statement below:

  /src/legacy.c (line 129)

   if( rc!=SQLITE_OK && rc==sqlite3_errcode(db) && pzErrMsg ){
    *pzErrMsg = malloc(1+strlen(sqlite3_errmsg(db)));
  
That's fine. But it's up to the user to free this memory. It
is not freed in sqlite3_close(db). To demonstrate this finding,
examine the report from valgrind.

  $ valgrind  --leak-check=yes  --tool=memcheck ./a.out \
             tmptable "drop table junk"


==24040== LEAK SUMMARY:
==24040==    definitely lost: 20 bytes in 1 blocks.
==24040==      possibly lost: 0 bytes in 0 blocks.
==24040==    still reachable: 128 bytes in 2 blocks.
==24040==         suppressed: 0 bytes in 0 blocks.


FIX

To fix the problem, add the following 3 lines to the example after
the fprintf statement.

 /* This will free zErrMsg if assigned */
    if (zErrMsg)
      free(zErrMsg);


Below is the complete example as I believe it should be


#include <stdio.h>
#include <sqlite3.h>

static int callback(void *NotUsed, int argc, char **argv, char **azColName){

  NotUsed=0;
  int i;
  for(i=0; i<argc; i++){
    printf("%s = %s\n", azColName[i], argv[i] ? argv[i] : "NULL");
  }
  printf("\n");
  return 0;
}

int main(int argc, char **argv){
  sqlite3 *db;
  char *zErrMsg = 0;
  int rc;

  if( argc!=3 ){
    fprintf(stderr, "Usage: %s DATABASE SQL-STATEMENT\n", argv[0]);
    exit(1);
  }
  rc = sqlite3_open(argv[1], &db);
  if( rc ){
    fprintf(stderr, "Can't open database: %s\n", sqlite3_errmsg(db));
    sqlite3_close(db);
    exit(1);
  }
  rc = sqlite3_exec(db, argv[2], callback, 0, &zErrMsg);
  if( rc!=SQLITE_OK ){
    fprintf(stderr, "SQL error: %s\n", zErrMsg);
    /* This will free zErrMsg if assigned */
    if (zErrMsg)
      free(zErrMsg);

  }
  sqlite3_close(db);
  return 0;
}

Now if this program is compiled and run with valgrind, you'll get the 
following output.

   $ valgrind  --leak-check=yes  --tool=memcheck ./a.out \
          tmptable "drop table junk"



==6510== LEAK SUMMARY:
==6510==    definitely lost: 0 bytes in 0 blocks.
==6510==      possibly lost: 0 bytes in 0 blocks.
==6510==    still reachable: 128 bytes in 2 blocks.
==6510==         suppressed: 0 bytes in 0 blocks.


Note the difference above.


Why This is Relevant:

Long running SQLite programs could cause potential memory leaks
with multiple failed calls, if the zErrMsg is not freed. It would 
be helpful to inform the user, with the first example, that it is
the programmers responsibility to free this variable.


Another example can be seen in the "SQLite Tutorial" in the
section C and C++ API. Reference the following link:

 http://souptonuts.sourceforge.net/readme_sqlite_tutorial.html


Regards,

Mike Chirico