Michael, You can simplify the patch. I ran the patched code under
valgrind and it informed me that there was a memory leak. It would
not be the end of the world, but it just isn't a good example.

Commands:

    $ make makefiles shared=no dynamicmaps=no
    ...
    $ cd src/util; make update
    ...
    $ cd ../global; make mail_dict
    ...
    $ cat /tmp/foo
    dbpath = /tmp/foo-db
    query = SELECT replacement FROM aliases WHERE mailbox = '%s'
    $ valgrind --tool=memcheck --leak-check=full --log-file=/tmp/valgrind/%p 
./mail_dict sqlite:/tmp/foo read
    ./mail_dict: error: sqlite:/tmp/foo: open database /tmp/foo-db: unable to 
open database file: No such file or directory
    ...
    get foo
    ./mail_dict: warning: sqlite:/tmp/foo is unavailable. sqlite:/tmp/foo: open 
database /tmp/foo-db: unable to open database file: No such file or directory
    foo: error
    ^D

Valgrind output fragment:

    ==1227729== 1,344 (824 direct, 520 indirect) bytes in 1 blocks are 
definitely lost in loss record 25 of 27
    ==1227729==    at 0x4843866: malloc (vg_replace_malloc.c:446)
    ==1227729==    by 0x567FD45: sqlite3MemMalloc.lto_priv.0 (sqlite3.c:26471)
    ==1227729==    by 0x567BC30: UnknownInlinedFun (sqlite3.c:30173)
    ==1227729==    by 0x567BC30: UnknownInlinedFun (sqlite3.c:30219)
    ==1227729==    by 0x567BC30: sqlite3Malloc.lto_priv.0 (sqlite3.c:30213)
    ==1227729==    by 0x572A8FE: UnknownInlinedFun (sqlite3.c:30498)
    ==1227729==    by 0x572A8FE: openDatabase (sqlite3.c:181410)
    ==1227729==    by 0x11EA41: dict_sqlite_open (dict_sqlite.c:334)
    ==1227729==    by 0x12A93D: dict_open3 (dict_open.c:492)
    ==1227729==    by 0x12A861: dict_open (dict_open.c:470)
    ==1227729==    by 0x13EA6B: dict_test (dict_test.c:88)
    ==1227729==    by 0x111127: main (mail_dict.c:121)

On closer investigation, the leak happens because you moved some
code out of dict_sqlite_close() into a new function dict_sqlite_free()
that does not free the db.

Simplified patch follows... please verify.

        Wietse

--- /var/tmp/postfix-3.10-20241202/src/global/dict_sqlite.c     2023-10-12 
11:34:40.000000000 -0400
+++ ./dict_sqlite.c     2025-01-03 11:15:36.752684221 -0500
@@ -59,6 +59,9 @@
 #if !defined(SQLITE_VERSION_NUMBER) || (SQLITE_VERSION_NUMBER < 3005004)
 #define sqlite3_prepare_v2 sqlite3_prepare
 #endif
+#if !defined(SQLITE_VERSION_NUMBER) || (SQLITE_VERSION_NUMBER < 3005000)
+#define sqlite3_open_v2(fname,ppDB,flags,zVfs) sqlite_open(fname,ppDB)
+#endif
 
 /* Utility library. */
 
@@ -320,10 +323,16 @@
     dict_sqlite->parser = parser;
     sqlite_parse_config(dict_sqlite, name);
 
-    if (sqlite3_open(dict_sqlite->dbpath, &dict_sqlite->db))
-       msg_fatal("%s:%s: Can't open database: %s\n",
-                 DICT_TYPE_SQLITE, name, sqlite3_errmsg(dict_sqlite->db));
+    if (sqlite3_open_v2(dict_sqlite->dbpath, &dict_sqlite->db,
+                       SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) {
+       DICT   *dict = dict_surrogate(DICT_TYPE_SQLITE, name, open_flags,
+                             dict_flags, "%s:%s: open database %s: %s: %m",
+                               DICT_TYPE_SQLITE, name, dict_sqlite->dbpath,
+                                     sqlite3_errmsg(dict_sqlite->db));
 
+       dict_sqlite_close(&dict_sqlite->dict);
+       return dict;
+    }
     dict_sqlite->dict.owner = cfg_get_owner(dict_sqlite->parser);
 
     return (DICT_DEBUG (&dict_sqlite->dict));
_______________________________________________
Postfix-devel mailing list -- postfix-devel@postfix.org
To unsubscribe send an email to postfix-devel-le...@postfix.org

Reply via email to