On 03/26/2012 10:55 AM, Dmitri Pal wrote:
On 03/26/2012 04:49 AM, Jan Zelený wrote:
> #0016:
> Ack
>
> #0017:
> I'm not sure what's the point in checking errno when malloc fails. IIRC the
> errno will be always ENOMEM. The same applies to the strndup several lines
> below.
>
> The initialization block (new_ctx->.... = NULL) is redundant, you set
> everything necessary later and in case of any error, uninitalized values won't
> make it out of the function anyway.
>
> #0018:
> Ack
>
> #0019:
> The return error at the end is misleading, please use return EOK instead
> (similar issue is in other patches as well, please change it when you see it).
>
> #0020:
> Please change the comment to:
> Determines if two file contexts are different by comparing:
>
> I don't like the prototype of ini_config_changed(). Is it necessary to return
> special error code? I would perform the check and in case of wrong input, I'd
> fall back to the safe option - return true (as in the config file has changed).
> In this case no special check is necessary and the code will be more readable.
>
> #0021:
> Ack
>
> #0022:
> I'm confused. In the patch comment you write that we can't remove it from the
> old interface but yet you remove it from the header file. I'd say it should
> remain there (and be marked) as well.
>
> #0023:
> Ack
>
> #0024:
> Ack
>
I am re-sending the whole set.
In #0015 I removed the trailing spaces Stephen noted in the other mail.
The rest 14 are the same.
#0017 - remove the initialization of the filename. Removing other
initialization is unsafe. Agreed on IRC.
Opened a ticket to do trust malloc to return ENOMEM.
https://fedorahosted.org/sssd/ticket/1276
I was told that malloc() by standard must return ENOMEM. Doe this
standard apply to all distros or just Linux?
#0019
Removed error variable and return EOK directly.
#0020
Changed comment.
Removed error variable and return EOK directly.
We agree on IRC that it is OK to keep function signature as is.
#0022
Unchanged.
Explanation:
Some time ago I started building a parallel interface in ini_configobj.h
to the existing public interface that is in ini_config.h So we do not
change the public existing one until the new one is ready and
stabilized. As one of the steps to prepare the new interface I renamed
the function in the new interface to be consistent with the naming
convention. I might provide some compatibility layer when I am ready to
switch from the old interface to the new interface but this is out of
scope of the current patch set.
Updated patch is attached.
The rest are same.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Sr. Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/