> Commit comments - the same comment in more patches is a little
confusing.
> Please leave only the part relevant for the patch.
Done.
> Patch #0002:
> - lines 45, 76, 96: the first part of condition is redundant
>
> (removed in patch #0006 - please merge that part with this patch)
Merging would be really difficult, so I'm leaving it separated if it's
not necessary.
I'd prefer if it wasn't but since all the patches will be applied for this to
work, I'm willing to overlook this ;-)
> - please use DEBUG_IS_SET in DEBUG_MSG and DEBUG
Done.
> Patch #0003:
> - line 166: I'd like to avoid changing these DEBUG statements like this
>
> -- either change them all or none, not just the one
> -- the same for lines 212, 228, 313
Done.
Also removed SSS_DEFAULT_DEBUG_LEVEL (replaced with SSSDBG_DEFAULT).
> Patch #0006:
> -<nitpick> please just for safety use parentheses in ternary operator
> (line 92 and couple more similar)</nitpick>
Done and also moved to macro.
I've created two new macros so the pattern to load debug_level from
command line is now:
/*
* Set debug level to invalid value
* so we can deside if -d 0 was used.
*/
INVALIDATE_DEBUG_LEVEL();
pc = poptGetContext(argv[0], argc, argv, long_options, 0);
while((opt = poptGetNextOpt(pc)) != -1) { ... }
CONVERT_AND_SET_DEBUG_LEVEL(debug_level);
> - why don't you remove SSS_UNRESOLVED_DEBUG_LEVEL entirely?
Removed (replaced with SSSDBG_UNRESOLVED).
You still don't have parentheses around the condition in ternary operator. No
big deal though, it doesn't concern me in the macro since bringing an error to
one possible place is less likely than to one of many possible places.
One more thing: I don't like the invalidate macro. The assignment with the
comment you provided is clear enough I think.
Other than this one small thing, the patches are ok.
Jan