[SSSD] [PATCH] Fix copying default dp_option values

Jakub Hrozek jhrozek at redhat.com
Wed Feb 26 14:32:41 UTC 2014


On Wed, Feb 26, 2014 at 02:24:44PM +0100, Pavel Březina wrote:
> On 02/26/2014 02:12 PM, Jakub Hrozek wrote:
> >On Wed, Feb 26, 2014 at 11:33:10AM +0100, Pavel Březina wrote:
> >>On 02/24/2014 05:39 PM, Jakub Hrozek wrote:
> >>>Hi,
> >>>
> >>>The attached two patches are related to:
> >>>https://fedorahosted.org/sssd/ticket/2257
> >>>
> >>>The first patch is included pretty much for completeness, as I noted
> >>>during development of the unit test, the blob type didn't handle any
> >>>default value.
> >>>
> >>>The second patch directly addresses #2257.  The previous code didn't
> >>>handle copying options well if the option was set to zero, because the
> >>>code followed logic like:
> >>>     if (oldval) {
> >>>         newval = oldval;
> >>>     else {
> >>>         newval = defval;
> >>>     }
> >>>
> >>>The patch implements Sumit's idea to provide a separate function for
> >>>copying default values and amend the generic copy function to only
> >>>create a copy using the current values.
> >>
> >>Nack.
> >>
> >>I think the approach is all right for this purpose, however have you
> >>considered creating a flag "value_set" instead?
> >
> >Yes, that was our first idea when we discussed the problem with Sumit.
> >But then he came up with the argument (which I agree with) that _copy
> >function should only really copy assigned values, not do a half-copy of
> >defaults and half of values..hence the split of copy and copy_defaults.
> 
> OK, this sounds reasonable.
> 
> >>
> >>I see the following warnings in your unit test:
> >
> >Ah, thank you, I didn't see them with clang, but they were reproducable
> >with GCC. Especially the first warning was bad and perhaps a compiler
> >bug (I suspect that clang optimizes the strlen for sizeof internally,
> >but I don't think it should do so with -O0)
> 
> Ack to both.

Pushed to master and sssd-1-11



More information about the sssd-devel mailing list