[SSSD] [PATCHES] Different patches for COLLECTION and INI

Dmitri Pal dpal at redhat.com
Fri Mar 12 15:37:19 UTC 2010


Stephen Gallagher wrote:
> On 03/11/2010 12:38 PM, Dmitri Pal wrote:
> > Hi,
>
> > Patch 1 - one-liner, turn off man page generation
> Ack.
>
> > Patch 2 - cleaning INI unit test to comply with formatting guidelines
> > and to turn off endless output in case of "make check".
> Ack.
>
> > Patch 3 - addressing ticket #170
> Nack. Please update the function names to reflect that the argument they
> take is uint32_t, etc
> e.g. col_add_int_property() -> col_add_int32_property()
>
> I am aware that you will need to update any places in the collection and
> INI where these functions are called.

Yes that would have a much bigger impact. I did not have to change the
unit test
since the type conversion is happening automatically. May be I should have?
I wanted to make changes compact as much as possible.
I used the same logic as Simo mentioned - the name of the function gives
a hint.
Do you insist on the change?
If you do I will do the change.

Also do you think I should change the unit test to use xxx32_t variables
inside tests functions instead of ints and longs?

>
> > Patch 4 - continuation of the work related on patch 3 regarding INI
> > interface that uses collection interface internally.
> Nack. (Nitpick)
> In the get_*_config_value() functions, if the return value of
> get_long_config_value() is outside the expected range, the correct error
> code to return should be ERANGE, not EIO.
>

I will reply both to you and Jakub here.
1) I agree with your comment and will change the error code.
2) As for the issue Jakub has noticed. Yes I am not 100% comfortable
with this too.
But I was trying to reduce the scope of the changes. It looks like the
right way would be
to create 4 separate functions instead wrapping one into another and use
strtoul, and strtoull.
But it is a more drastic change that I was trying to avoid.
Let me know if you think that this change is justified at the moment. If
you think it is the patch will be underway.
 

Now should we then change the names of the functions for the INI too.
If we change the collection code names why not change both?

Please let me know how I should proceed.

Thanks for review!

>
>
>
_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




More information about the sssd-devel mailing list