Since directly testing the return value is easy enough, we should do that
rather than test other things that *should* mean that the return value is
non-NULL. Based on some grepping, literally 55% of calls to db_get_result()
are passed to strtoull() within the next two lines of code. 11 calls are on
the same line of code (presumably waiting to die when db_get_result() returns
a NULL for whatever reasons it might -- no data, db error, etc.)
[dbmail-2.0rc2]$ grep db_get_result db.c | wc
67 298 2930
[dbmail-2.0rc2]$ grep db_get_result db.c | grep strtoull | wc
11 64 645
[dbmail-2.0rc2]$ grep db_get_result -A1 db.c | grep strtoull | wc
19 132 1194
[dbmail-2.0rc2]$ grep db_get_result -A2 db.c | grep strtoull | wc
37 234 2143
[dbmail-2.0rc2]$ grep db_get_result -A10 db.c | grep strtoull | wc
37 234 2143
Since it's only 37 cases, I'll go through and look to see if any of them have
special handling for the NULL case, or if they just use 0. If it turns out
that way, then I'll write up a db_get_result_u64() function.
Aaron
Ilja Booij <[EMAIL PROTECTED]> said:
> Hi,
>
> I wouldn't now actually.. I guess there are functions for which strtoull
> can return a 0.
>
> auth_getmaxmailsize() is an example of this. When the maxmail_size for a
> user is 0, strtoull will return 0.
>
> I agree with the fact that we need to clean up the code in db.c to deal
> with db_get_result() returning NULL.
>
> Maybe we should pay more attention to the reason why db_get_result()
> returns 0.
>
> In dbpgsql.c/dbmysql.c, db_get_result() will return 0 if the result set
> is NULL, of if the row, field combination asked is not in the result
> set. Before every db_get_result() we could make sure we are only asking
> for a valid row, by making sure the row_nr we are asking for is <
> db_num_rows().
> db_num_rows() will return 0 if there are no rows in the reslt set. If
> this is the case, no call to db_get_result() should be done.
>
> Still, I guess we should make sure that no call will be made to
> strtoull() when the query_result is NULL.
>
> Ilja
>
> Aaron Stone wrote:
>
> > It occurs to me that the correct solution really is to have a function that
> > integrates a properly validated strtoull() call, but what I can't figure out
> > is how to determine the difference between a 0 and a NULL result -- that is,
> > if we get a NULL within the function, and return 0, it's no different than
> > actually getting 0. Are there potential consumer functions that need to
> > differentiate these situations?
> >
> > Aaron
> >
> >
> > "Aaron Stone" <[EMAIL PROTECTED]> said:
> >
> >
> >>I just got a crash while trying to make a delivery to a non-existant user. I
> >>skipped past the part that made sure that the user exists and so was able to
> >>discover a pretty significant bad idiom that's widely used in db.c
> >>
> >>The problem is that db_get_result() can return NULL if there's actually no
> >>results to find. In a number of cases, the count of returned rows in not
> >>checked, db_get_result() is called, and it is not tested for a valid return
> >>before being derefenced or passed as an argument to a standard library
> >>function, such as strtoull(), that apparently cannot handle a NULL first
> >
> > argument.
> >
> >>In a few places there is a better idiom (this is from db_createsession():
> >>
> >> tmpmessage.msize =
> >> query_result ? strtoull(query_result, NULL, 10) : 0;
> >>
> >>db.c should really be audited and this better idiom put wherever possible.
> >>Note that it is entirely possible to have a situation in which even after
> >>checking everything that indicates that a value should be returned,
> >>db_get_result() can still return a NULL value; it's output should always be
> >>checked!
> >>
> >>Alternatively, since at least half of the calls to db_get_result() are fed
> >>into strtoull() right away, why not have a function that does the proper
> >>result validation and then makes the call to strtoull() and returns a u64_t?
> >>
> >>Aaron
> >>_______________________________________________
> >>Dbmail-dev mailing list
> >>Dbmail-dev@dbmail.org
> >>http://twister.fastxs.net/mailman/listinfo/dbmail-dev
> >>
> >
> >
> >
> >
> _______________________________________________
> Dbmail-dev mailing list
> Dbmail-dev@dbmail.org
> http://twister.fastxs.net/mailman/listinfo/dbmail-dev
>
--
