On 08/23/10 07:26 AM, Matt Keenan wrote:
Thanks Jan for the review, I've updated the webrev to contain your
recommendations. All of which I agree with :-)
New webrev available at :
http://cr.opensolaris.org/~mattman/bug-14004/
td_mg.c, 358-384: I realize that the algorithm you're using here of
count, realloc, then walk & copy is the same as used for several other
object types here, but it's really not a great design. It would be
great if we would fix it at least for this case, but would you at least
file a bug against libtd to have the design reconsidered so that the
discovery calls return a count to start with, rather than callers having
to re-count the number of objects returned?
td_zpool.c
84: unnecessary NULL test
302, 364, 379, 520: "allocate"
367: leaked top_target->name
381: seems like this would leak memory from prior iterations if this
failed after the first. It'd also leak top_target->targets and
top_target->name on any failure.
522,537,563,595,626: would this leak at least zi? 537 and later returns
also seems to leak zi->attributes.targets, if not others. recommend a
goto to a general cleanup and failure handler for this function, as
there's a lot of allocation going on. seems like use of zpool_info_free
would help?
884,1024,1069,1114,1159: no error if == NULL? the number of times this
idiom seems to be repeated indicates that there's perhaps a stronger
utility function lurking
Dave
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
