Hi
Aaron Stone wrote:
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.)
You're right. Otherwise a new error is introduced way to easily. :)
[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.
Great, sounds like a plan. :D
Ilja
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
