On Wed, Dec 5, 2018 at 9:59 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Fri, Nov 30, 2018 at 10:24 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > This moves handling of initial SIDs into a separate table. Note that the > > SIDs stored in the main table are now shifted by SECINITSID_NUM and > > converted to/from the actual SIDs transparently by helper functions. > > > > This change doesn't make much sense on its own, but it simplifies > > further sidtab overhaul in a succeeding patch. > > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > > --- > > security/selinux/ss/policydb.c | 10 +- > > security/selinux/ss/services.c | 88 +++++++++-------- > > security/selinux/ss/services.h | 2 +- > > security/selinux/ss/sidtab.c | 168 ++++++++++++++++++++------------- > > security/selinux/ss/sidtab.h | 15 ++- > > 5 files changed, 173 insertions(+), 110 deletions(-) > > Based on a quick look, this looks fine to me, and you've gone a couple > rounds with Stephen giving it the okay so I'm merging this into > selinux/next, but I've had to do some minor tweaks to make > checkpatch.pl happy (line length) and cleanup the whitespace (no > double vertical whitespace). > > Please start running checkpatch.pl on your patches. I don't care if > you run it via a git hook, by hand, or some other method - just do it. > It isn't hard and it catches lots of silly mistakes. I believe this > is the second time I've mentioned this to you during this development > cycle, if this continues I'm going to start rejecting your patches if > they fail checkpatch.pl. > > I'll be the first to admit that checkpatch.pl isn't perfect, e.g. > sometimes fixing lines to 80-chars can make things worse, so if you > have any questions about checkpatch.pl errors please ask (preferably > on-list so everyone can benefit). I did run checkpatch.pl and I fixed all ERRORs reported. I also fixed most WARNINGs, leaving only line length warnings that I wasn't sure were worth fixing. Documentation/process/submitting-patches.rst [1] does leave a room for exceptions and I know that at last some maintainers strongly prefer slightly longer lines before awkward line breaks (and I can see that you left a couple 80+ lines in the final commit of the second patch as well). I took a bet on leaving most of the lines as-is and now I see it was a bad choice... I'll tweak the imaginary knob further in the strict direction, hopefully I'll have a better accuracy next time. As for the double empty line, I agree it shouldn't be there, but I didn't spot it because chachpatch.pl isn't complaining about that at all on my side. Did you spot it manually or am I missing some checkpatch.pl flag? Thank you for still having patience with me and thanks to Stephen for the reviews! I hope the patches will cause more good than bad from now on :) [1] https://www.kernel.org/doc/html/v4.19/process/submitting-patches.html#style-check-your-changes > [...] -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.
