Обсуждение: Source code cleanup

I'm getting a bunch of compiler warnings when I compile the latest
psqlodbc sources from CVS. I spent some time cleaning those up.

I also got access to a report of a Coverity scan over psqlodbc source
tree. That found a bunch of additional issues, some of which were
genuine (minor) bugs. I'm still in the process of going through the
report, but I fixed some bugs already.

I also wrote a small regression test suite to test with. It only tests a
few of the ODBC functions at the moment, but it's better than nothing,
and it can be easily extended. What do others use for testing psqlodbc?

I pushed the result to a git repository at
git@github.com:hlinnaka/psqlodbc.git. It contains the psqlodbc CVS
history, as converted by git cvsimport, and my changes as commits on top
of that. I can also send the changes as patches if that's preferred. Let
me know how to proceed.

- Heikki

Hi Heikki,

(2013/01/09 3:25), Heikki Linnakangas wrote:
> I'm getting a bunch of compiler warnings when I compile the latest
> psqlodbc sources from CVS. I spent some time cleaning those up.
>
> I also got access to a report of a Coverity scan over psqlodbc source
> tree. That found a bunch of additional issues, some of which were
> genuine (minor) bugs. I'm still in the process of going through the
> report, but I fixed some bugs already.

Thanks.

>
> I also wrote a small regression test suite to test with. It only tests a
> few of the ODBC functions at the moment, but it's better than nothing,
> and it can be easily extended. What do others use for testing psqlodbc?
>
> I pushed the result to a git repository at
> git@github.com:hlinnaka/psqlodbc.git. It contains the psqlodbc CVS
> history, as converted by git cvsimport, and my changes as commits on top
> of that.

I would check the git repository.

 > I can also send the changes as patches if that's preferred. Let
> me know how to proceed.
>
> - Heikki

regards,
Hiroshi Inoue

Hi Heikki,

(2013/01/10 12:58), Inoue, Hiroshi wrote:
> Hi Heikki,
>
> (2013/01/09 3:25), Heikki Linnakangas wrote:
>> I'm getting a bunch of compiler warnings when I compile the latest
>> psqlodbc sources from CVS. I spent some time cleaning those up.
>>
>> I also got access to a report of a Coverity scan over psqlodbc source
>> tree. That found a bunch of additional issues, some of which were
>> genuine (minor) bugs. I'm still in the process of going through the
>> report, but I fixed some bugs already.
>
> Thanks.
>
>>
>> I also wrote a small regression test suite to test with. It only tests a
>> few of the ODBC functions at the moment, but it's better than nothing,
>> and it can be easily extended. What do others use for testing psqlodbc?
>>
>> I pushed the result to a git repository at
>> git@github.com:hlinnaka/psqlodbc.git. It contains the psqlodbc CVS
>> history, as converted by git cvsimport, and my changes as commits on top
>> of that.
>
> I would check the git repository.

The 1st question I have is about the change

commit 80b3eb35ab1428949f7ab8fbd567d5d49655dc02
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:   Sat Jan 5 00:35:39 2013 +0200

     Silence Coverity warnings about passing negative param to
    SQLExecDirect.
     It doesn't know about the psqlodbc extension to that function that
      passing SQL_NTS means it's a C-style null-terminated string.

The type of the 3rd parameter of SQLExecDirect() is SQLINTEGER.
What kind od warnings does Coverity scan show?
SQL_NTS is generally used in ODBC and I could hardly remember the
examples which use parameters other than SQL_NTS.

In addition the change doesn't seem correct to me.
The 3rd parameter of the following SQLExecDirect call is the
string length of a Statement Handle.

                  ret = SQLExecDirect(stmt, (SQLCHAR *) "select gid from
pg_prepared_xacts", strlen(stmt));


regards,
Hiroshi Inoue

(cc'ing pgsql-odbc mailing list yet again, I misspelled it earlier..)

On 22.02.2013 00:39, Hiroshi Inoue wrote:
> HI Heikki,
>
> (2013/02/21 17:55), Heikki Linnakangas wrote:
>> (cc'ing psql-odbc mailing list again, got dropped by accident)
>>
>> Hi Hiroshi,
>>
>> Any news on this? Would you like me to prepare a new set of patches with
>> those things fixed?
>
> Sorry for the late reply.
> I'm happy if you make new set of patches.

Ok, I fixed the couple of bugs you mentioned, rebased, and pushed to the
git repository again. See master branch at
git@github.com:hlinnaka/psqlodbc.git. Link to the github web interface:
https://github.com/hlinnaka/psqlodbc.

I separated the regression test suite I wrote to a separate branch,
master-with-testcases, for easier review. There's also a third branch in
the repository, "upstream", which is a mirror of the current CVS tip.

I kept the change that turns the switch- to if-statement in
getPrecisionPart() for now
(https://github.com/hlinnaka/psqlodbc/commit/1dd3bfb6e6a7f5d1bef49103593178ccc2caf129).
You suggested adding a default-case to that instead, which is fine with
me as well so feel free to change it at commit, although I find it
slightly more readable with "if".

- Heikki