Re: [Bacula-devel] PostgreSQL issues - Job.{clientid | starttime | endtime}

[Kern S. <ke...@si...>]

On Tue, 2003-12-30 at 14:08, Dan Langille wrote:
> On 29 Dec 2003 at 9:53, Kern Sibbald wrote:
> 
> > Hello Dan,
> > 
> > First, thanks for digging into this ...
> > 
> > On Sun, 2003-12-28 at 22:30, Dan Langille wrote:
> > > I'm looking through the remaining issues raised during the PostgreSQL 
> > > port.  These are documented at:
> > > 
> > >    http://www.freebsddiary.org/bacula/known-issues.php
> 
> [snip]
> 
> > > I have grep'd the code for references to StartTime.  I think we have 
> > > to look carefully at the following items:
> > > 
> > > src/cats/sql_get.c line 290:
> > > 
> > >   bstrncpy(jr->cStartTime, row[3]!=NULL?row[3]:"", 
> > >        sizeof(jr->cStartTime));
> > > 
> > > This assumes a null value from the database == NULL.  It should also 
> > > check for an emtpy string.  I haven't tested this.  Similarly for the 
> > > next line which references EndTime.
> > > 
> > > ACTION ITEM:  What needs to be done here?
> > 
> > The check for NULL is a good idea here, and if PostgreSQL returns ""
> > that is fine to, because it is what a NULL is converted into. The main
> > idea is to prevent references to a null pointer. When turning cStartTime
> > (character StartTime) into internal Unix time, appropriate checks will
> > be done.
> 
> We must be careful with our terminology here.  The code is testing 
> for NULL (0), not a null value (database).  
> 
> For a null value (database), PostgreSQL will give us an empty string, 
> according to the documentation for PQgetvalue (which is the function 
> we are using).  See 
> http://www.postgresql.org/docs/current/static/libpq-exec.html  
> 
> "An empty string is returned if the field value is null. See 
> PQgetisnull to distinguish null values from empty-string values."  
> 
> The code above checks to see if the row[3] != 0, not an empty string. 
>  If an empty string is found, it gets copied to jr->cStartTime.   
> 
> I think the code needs to be changed to (I have not tested this):
> 
>  bstrncpy(jr->cStartTime, 
>                row[3] != NULL || row[3] != "" ? row[3] : "",   
>                sizeof(jr->cStartTime));

Terminology: yes, I agree. Whenever I say a NULL, I mean a NULL pointer.
I will never use it in any other context. NULLs are what concern me
because they can lead to seg faults. I think we have mostly covered
them.  An empty string (or null string as opposed to NULL) is generally
OK with Bacula as it normally does checking, and at least the program
doesn't usually crash on an empty string.  It is interesting that
PostgreSQL returns and empty string for a null DB value. MySQL also
returns an empty string if I remember right, but SQLite used to always
return a NULL.

I think we are reasonably covered against crashes. The only thing for
you to check is to ensure that a null time is handled correctly in
Bacula for PostgreSQL. I'll be happy to make whatever changes are
necessary -- especially because I believe we are down to the fine tuning
stage :-)

Regards, Kern