Thread: Static analysis run

Hi,

We use psqlodbc in one of our projects at my company and we recently
bought a static source code analysis program (Klocwork) to run on our
own software. Since we use psqlodbc, I also ran it on that and it
found a few things. This was run on the CVS head branch from this
today (fri 22 july).

Attached is a diff with some fixes that I managed myself. Then I also
have some places that were not as easy for me to fix, but maybe some
of you that know the code better can do it. Here is a list with
things, some might not be correct, since this tool sometimes thinks
there's an error where there aren't. Row numbers might not be exact
because of some local changes, but they should be almost right.


 convert.c in ResolveOneParam:2651 allocbuf is allocated, further down
 (2785), CVT_APPEND_CHAR and CVT_APPEND_STR are used a lot, and these
 macros may return from the function, giving a memory leak for
 allocbuf.

 environ.c in PGAPI_FreeEnv:89 env is freed even if EN_Destructor
 returns false above, which means it's accessing freed memory.

 info.c in PGAPI_Statistics:2623 If SC_MALLOC_return_with_error
 returns, column_names allocated above is lost.

 odbcapi30w.c in SQLGetDescFieldW:128 blen is not initialised before
 sent to utf8_to_ucs2(). The same thing exists in SQLColAttributeW:287
 and SQLGetDiagFieldW:345.

 results.c in SC_pos_reload_needed:2189 rows_per_fetch is
 uninitialised, if create_from_scratch is false.


These are the things that I saw just looking through the report pretty
quickly. There are a lot of reports about dereferenced NULL pointers,
but I haven't looked more closely at any of those. Most of them comes
from not checking what malloc and family returns. It also complains a
lot about strcpy and sprintf not checking boundaries. Those might not
at all be errors, but then again they could cause buffer overflows.

If you are interested, I can send the full report (200KB html, 295
entries including the ones in the patch and the above things), and if
someone has time, that person can go through the list.


Greetings,

Tomas

Thanks Tomas,

I'll review the patch and apply it as soon as I get some time. I would be interested in a copy of the full report if
youdon't mind. 

Thanks, Dave.

> -----Original Message-----
> From: pgsql-odbc-owner@postgresql.org
> [mailto:pgsql-odbc-owner@postgresql.org] On Behalf Of Tomas Skäre
> Sent: 22 July 2005 16:19
> To: pgsql-odbc@postgresql.org
> Subject: [ODBC] Static analysis run
>
> Hi,
>
> We use psqlodbc in one of our projects at my company and we recently
> bought a static source code analysis program (Klocwork) to run on our
> own software. Since we use psqlodbc, I also ran it on that and it
> found a few things. This was run on the CVS head branch from this
> today (fri 22 july).
>
> Attached is a diff with some fixes that I managed myself. Then I also
> have some places that were not as easy for me to fix, but maybe some
> of you that know the code better can do it. Here is a list with
> things, some might not be correct, since this tool sometimes thinks
> there's an error where there aren't. Row numbers might not be exact
> because of some local changes, but they should be almost right.
>
>
>  convert.c in ResolveOneParam:2651 allocbuf is allocated, further down
>  (2785), CVT_APPEND_CHAR and CVT_APPEND_STR are used a lot, and these
>  macros may return from the function, giving a memory leak for
>  allocbuf.
>
>  environ.c in PGAPI_FreeEnv:89 env is freed even if EN_Destructor
>  returns false above, which means it's accessing freed memory.
>
>  info.c in PGAPI_Statistics:2623 If SC_MALLOC_return_with_error
>  returns, column_names allocated above is lost.
>
>  odbcapi30w.c in SQLGetDescFieldW:128 blen is not initialised before
>  sent to utf8_to_ucs2(). The same thing exists in SQLColAttributeW:287
>  and SQLGetDiagFieldW:345.
>
>  results.c in SC_pos_reload_needed:2189 rows_per_fetch is
>  uninitialised, if create_from_scratch is false.
>
>
> These are the things that I saw just looking through the report pretty
> quickly. There are a lot of reports about dereferenced NULL pointers,
> but I haven't looked more closely at any of those. Most of them comes
> from not checking what malloc and family returns. It also complains a
> lot about strcpy and sprintf not checking boundaries. Those might not
> at all be errors, but then again they could cause buffer overflows.
>
> If you are interested, I can send the full report (200KB html, 295
> entries including the ones in the patch and the above things), and if
> someone has time, that person can go through the list.
>
>
> Greetings,
>
> Tomas
>
>
>

> -----Original Message-----
> From: pgsql-odbc-owner@postgresql.org
> [mailto:pgsql-odbc-owner@postgresql.org] On Behalf Of Tomas Skäre
> Sent: 22 July 2005 16:19
> To: pgsql-odbc@postgresql.org
> Subject: [ODBC] Static analysis run
>
> Hi,
>
> We use psqlodbc in one of our projects at my company and we recently
> bought a static source code analysis program (Klocwork) to run on our
> own software. Since we use psqlodbc, I also ran it on that and it
> found a few things. This was run on the CVS head branch from this
> today (fri 22 july).
>
> Attached is a diff with some fixes that I managed myself.

Thanks, patch applied.

Regards, Dave.