Re: adding raises (SQLException) to two methods of published XDatabaseMetaData ?

On Fri, Jun 13, 2014 at 11:46:07AM +0100, Caolán McNamara wrote:
> in published api

> offapi/com/sun/star/sdbc/XDatabaseMetaData.idl

> two methods...

>     long getDriverMajorVersion();
>     long getDriverMinorVersion();

> have no raises (SQLException) spec unlike all the other members,

That's inherited from JDBC, of which SDBC is basically a C++ "port".

> and the following four coverity ids show that there are 4
> implementations of these that actually could throw SQLException

I'd rather fix those.

> 706317 Uncaught exception
>   line 1215 connectivity/source/drivers/jdbc/DatabaseMetaData.cxx
>         sal_Int32 SAL_CALL
> java_sql_DatabaseMetaData::getDriverMajorVersion(  )

This one will throw SQLException only if JNI fails to find a
"getMajorDriverVersion" method in the JDBC driver (the Java code we
make calls to). This comes from file
connectivity/source/drivers/jdbc/Object.cxx
method java_lang_Object::obtainMethodId

1) In this particular case, I don't think this will happen (assuming
   JNI as a whole functions OK) since it is part of the JDBC
   interface. Doesn't Java check that an implementation that claims to
   implement the interface has the right methods with the right
   signature? I think it does.

   Obviously, Coverity cannot see that.

2) Frankly, I'm not sure SQLException is the right exception. It could
   be for other methods than "getMajorDriverVersion" that would be
   optional in some way? If that is the case, then, well, the code
   cannot distinguish between these two cases.

> 706361 Uncaught exception

That's in ODBC. It will throw (from
connectivity/source/drivers/odbc/OTools.cxx
method OTools::ThrowException) only if we make the ODBC call with an
invalid handle. I'm not aware of all scenarios out of the top of my
head, but there is a good possibility that it happens only if the
LibreOffice ODBC code deeply screws up *internally* by giving a
different handle than what it was given by the ODBC driver, or by
using a handle after it was freed, ... That is, it is not an error of
the calling code, and throwing an SQLException to the calling code is
maybe not the best choice. I'm a bit undecided on that.

> 706318 Uncaught exception

Same as 706317

> 706362 Uncaught exception

Same as 706361


So, one question is, what is the LibreOffice "coding standard" for
"internal inconsistency"? Do we have a specific exception for that, do
we rather abort() (in debug mode) and return "no value / empty value /
0" to the caller?

-- 
Lionel