On 1.2.2016 09:03, Jan Cholasta wrote: > Hi, > > On 29.1.2016 15:49, Martin Basti wrote: >> >> >> On 29.01.2016 15:49, Stanislav Laznicka wrote: >>> Reworded the commits so that they better reflect what's going on in those. >>> >>> On 01/29/2016 02:49 PM, Stanislav Laznicka wrote: >>>> Hello, >>>> >>>> I made some patches based on the Coverity report from 18.1.2016. >>>> >>>> Cheers, >>>> Standa >>>> >>>> >>> >>> >>> >> NACK, see my previous email > > I don't think this deserves 9 patches, 1 would be sufficient enough.
I would rather have it is separate patches as these fixes are largely not related. It will make bisecting easier. > Patch 0013: > > 1) I think this unreachable return is intentional, as indicated by the > comment: > > - #we shouldn't get here > - return [UNKNOWN_ERROR] I would use assert False, "we shouldn't get here" neither we nor Coverity are confused when we hit the code path one day. UNKNOWN_ERROR would pop up somewhere else and it will be harder to find out why the hell the code behaves as mad. Traceback will clearly indicate that there is a problem with the 'switch'. Petr^2 Spacek > 2) How is this dead code? > > - if options.mode == 'validate_pot' or options.mode == 'validate_po': > + if options.mode in ('validate_pot', 'validate_po'): > > - elif options.mode == 'create_test' or 'test_gettext': > + elif options.mode in ('create_test', 'test_gettext'): > > > > Patch 0014-0015: LGTM > > > Patch 0016: The original code is in fact correct. > > > Patch 0017: This will break Python 3. The two branches are performing the same > action, but with different data types. > > > Patch 0018: LGTM > > > Patch 0019: IMO the original code is better (None has a __class__ too, you > know). > > > Patch 0020: LGTM > > > Patch 0021: Please use the original error messages (there are no requests > being added to D-Bus, but to certmonger). > > > Honza -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code