-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/23/2009 07:09 PM, John Engelhart wrote:
> While the top of ares_parse_srv_reply() checks the srv_out and nsrvreply
> arguments to make sure they are non-NULL before using them, the bottom
> of the function does not:
>
> ares_parse_srv_reply(const unsigned char *abuf, int alen, struct
> srv_reply **srv_out, int *nsrvreply)
> {
> // ... snip ... Near top, line ~66
>
> /* Set *srv_out to NULL for all failure cases. */
> if (srv_out)
> *srv_out = NULL;
> /* Same with *nsrvreply. */
> if (nsrvreply)
> *nsrvreply = 0;
>
> // ... snip ... Near bottom, line ~165
>
> /* everything looks fine, return the data */
> *srv_out = srv;
> *nsrvreply = ancount;
>
> // ... snip ...
> }
>
> Those bottom lines should probably be:
>
> /* everything looks fine, return the data */
> if(srv_out) *srv_out = srv;
> if(nsrvreply) *nsrvreply = ancount;
>
Hi,
after looking once again how the checks are done in other ares
functions, I think the if()s at top are not necessary - I just NULL the
pointers unconditionally similar to how ares_parse_ptr_reply() and
ares_parse_ns_reply() work.
I'm aware that with srv_out == NULL you get a crash -- I'm still not
sure which way of coding is preferred within c-ares, be it the more
defensive one of ares_parse_{a,aaaa}_reply or the one of ptr/srv. If a
very defensive way of checking is preferred, I think the parsing
function should just return something like ARES_EINVAL or similar..
Jakub
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkrlyWwACgkQHsardTLnvCV1lwCeMDTesCYmzF9bxgnGeaDdWAoZ
bFYAoJAZHSdqErDBhCzDzBNAWKPf5UKW
=yhYr
-----END PGP SIGNATURE-----