[SSSD] [PATCHES] New functionality for refarray and new comment object

Stephen Gallagher sgallagh at redhat.com
Tue Apr 27 00:18:23 UTC 2010


On 04/26/2010 04:38 PM, Dmitri Pal wrote:
> Hello,
>
> Patch 1: New functionality for refarray. Some basic functionality was
> missing. Now it is added.

Nitpick: in ref_array_replace(), your trace message lists 
ref_array_insert().  It might not be a bad idea in a separate patch to 
change TRACE_FLOW_* to use __FUNCTION__ instead of passing it the 
function name explicitly. It's less error-prone.

In ref_array_reset(), you need to document very clearly the order that 
the callbacks will be invoked or it needs to be documented that the 
order is not guaranteed. I think the latter is probably a better move, 
since it grants us leeway in the future, as well encouraging the 
consumer not to right order-dependent callbacks.


>
> Patch 2: New object to preserve and store comments in the INI file.
> Takes advantage of the refarray interface.
> This patch includes a separate unit test. Nothing uses this object yet.
> It will be a part of the new more complex value object I am currently
> working on.

I'd prefer ini_comment_is_valid() rather than ini_comment_is_val(). 
'val' usually means 'value', so this is confusing.

Also, I'd have to check with RFC822 on this, but I think leading spaces 
followed by ; or # should still be a valid comment.

Please reduce indentation on the mode handling in ini_comment_modify(). 
It's much too far right. Our coding guideline specifies four spaces.

Otherwise, this looks good to me.



-- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



More information about the sssd-devel mailing list