Comment 1 for bug 457442

Revision history for this message
root (n-root-psiphon-ca) wrote :

Here's a quick, first cut using a basic tool, cppcheck. Also see the item at the bottom which is from compiler warnings.

                cppcheck output:

                [./mod_psiphon_db_to_cookie.c:100]: Memory leak: tm
                [./mod_test.c:245]: Resource leak: f
                [./proxy/proxy_util.c:2885]: Memory leak: tm

                Here are the files. Review my fixes in rev 967.

                =======================================================

                ./mod_psiphon_db_to_cookie.c

                No frees on return, so this does look like a memory leak. What's sizeof(*tm), shouldn't it be sizeof(struct tm)? Wait, why is there am allocation here at all? It looks like the pointer to the allocated memory is overwritten ("tm = gmtime(&t)") and the allocated memory is never referenced.. Also, should use thread-safe gtime_r (http://www.opengroup.org/onlinepubs/009695399/functions/gmtime.html).

                =======================================================

                struct tm *tm = malloc(sizeof(*tm));
                time_t t = time(NULL);
                char *tstr;

                dbtc_log("dbtc()");

                dconf = (struct_dbtc_dir_config *) ap_get_module_config(r->per_dir_config,
                &psiphon_db_to_cookie_module);

                if (!dconf->need_dbtc) {
                dbtc_log("dbtc(): no need dbtc");
                return DECLINED;
                };

                if (r->main != NULL) /*no subrequests*/
                {
                dbtc_log("dbtc(): not a main request");
                return DECLINED;
                }
                tm = gmtime(&t);
                t = mktime(tm);
                tstr = apr_psprintf(r->pool, "%d", t);

                =======================================================

                ./mod_test.c

                File f isn't closed. Is this just a test routine? Should fix anyways. Should this file be removed before open source?

                =======================================================

                int test_hook(request_rec *r) {
                struct_test_dir_config *dconf;
                char body[MAX_BODY];
                // const char *body = "here is line\r\nhere IS digits: 12345\r\nqqqq\n";
                const char *result;

                regs_v_t **regs_v;
                FILE *f;

                jsf_log("test_hook()");

                dconf = (struct_test_dir_config *) ap_get_module_config(r->per_dir_config,
                &test_module);

                if (!dconf->enable) {
                jsf_log("test_hook(): don't need test");
                return DECLINED;
                };

                jsf_log("test_hook(): running");

                ap_set_content_type(r, "text/plain");

                if (!(f = fopen("/var/tmp/test.js", "r"))) {
                jsf_err_log("test_hook(): Can't open a file");
                return OK;
                }

                regs_v = jsf_init1(r);
                if (!regs_v) jsf_err_log("test_hook(): no regs_v");

                /*
                while (!feof(f)) {
                if (fgets(body, MAX_BODY, f)) {
                if (regs_v) {
                result = jsf(r, regs_v, body, strlen(body));
                } else {
                result = body;
                }

                ap_rprintf(r, "%s", result);
                }
                }

                fclose(f);
                */
                return OK;
                }

                =======================================================

                ./proxy/proxy_util.c

                Same as 1st case, memory leak.

                =======================================================
                struct tm *tm = malloc(sizeof(*tm));
                time_t t = time(NULL);
                char *tstr;

                ctdb_log("ap_proxy_cookie_to_db(\"%s\")", val);

                tm = gmtime(&t);
                t = mktime(tm);
                tstr = apr_psprintf(r->pool, "%d", (int)t);

                if (!(dbd = ap_dbd_acquire(r))) {
                ctdb_err_log("ctdb(): Can't get DB connect");
                return;
                }
                =======================================================

                From compiler warnings:

                mod_map_to_proxy.c: In function ‘hook_uri2proxy’:
                mod_map_to_proxy.c:166: warning: too few arguments for format

                if(!*decrypted_uri)
                {
                if (ap_is_default_port(port, r))
                {
                return_url=apr_psprintf(r->pool, "%s://%s%s/", ap_http_scheme(r),
                ap_get_server_name(r), conf->loc);
                }
                else
                {
                return_url=apr_psprintf(r->pool, "%s://%s:%u%s/", ap_http_scheme(r),
                ap_get_server_name(r), port);
                }

                apr_table_setn(r->err_headers_out, "Location", return_url);
                return HTTP_MOVED_TEMPORARILY;

                }