On 02/14/2014 04:19 AM, Lukas Slebodnik wrote:
On (13/02/14 13:40), Dmitri Pal wrote:
> On 02/13/2014 11:07 AM, Lukas Slebodnik wrote:
>> +void print_config_parsing_errors(FILE *file,
>> + struct collection_item *error_set);
>> +
>> +
>> +void print_file_parsing_errors(FILE *file,
>> + struct collection_item *error_list);
> Seems like there is an extra tab or space on the second line.
> I did a visual review. The patches otherwise look fine.
> I have not tried to apply them though.
>
Fixed.
Updated version is attached.
LS
I applied patches, however I get warnings.
They might have been there, but since it is the area that you are trying
to clean it might make sense to not have these warnings:
[dpal@dpal ding-libs (master)]$ make check | grep warning
collection/collection_cnv.c:133: warning: no previous prototype for
‘col_insert_unsigned_property’
collection/collection_cnv.c:387: warning: no previous prototype for
‘col_insert_unsigned_property_with_ref’
I remember a conversation about this issue...
I think we wanted to fix collection.h to have the right prototypes.
Fixing the prototypes is an ABI change however the functions are not
usable so no one uses it this way, thus I think it is safe just to fix:
s/unsinged/unsigned
in collection.h
ini/ini_config_ut.c:309: warning: ‘negative_test’ defined but not used
adding
(error = negative_test()) ||
into ini_config_ut.c at line 1587 fixes the problem. I know we talked
about it. I now took a deeper look. I seems that you are right and there
is really not harm in adding this test.
Otherwise ack. The patch to fix these things can be provided separately
on top of the patches you already sent.
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/