On Wednesday, March 27, 2013 01:35:52 PM vi0oss wrote:
> On 03/26/2013 11:54 PM, Paul Moore wrote:
> > On Thursday, March 21, 2013 08:51:34 PM [email protected] wrote:
> >> From: Vitaly _Vi Shukela<[email protected]>
> >
> > First off, a general comment, can I have your permission to add your
> > sign-off to these patches?
>
> Yes.
Okay, great.
> Should I just press "Sign off" button by default when submitting
> code to public open source projects?
Every project is different, I'd recommend looking at the mail archives to see
how others submit patches successfully. You can learn a lot by reading mail
archives ...
> > Also, I'll go ahead and fix them up but in the future
> > submissions please be more mindful of whitespace, alignment, and the 80
> > character width limit.
>
> Strange. This time I explicitly checked the whitespace (there were many
> errors, I used vim to make tabs vs spaces look like the rest of the
> code, than I used Git to fix up "trailing whitespace" on lines and only
> after that I sent). I also remember looking at 80-character line and
> fixing overrun lines...
>
> Is there some /usr/bin/indent call to automatically make all whitespace
> just right for this project?
Not that I am aware of, but with the exception of the Python bindings (they
follow the Python conventions), the libseccomp code tends to follow the Linux
Kernel conventions.
When in doubt, always follow the existing conventions in the file you are
editing; that rule almost always works, regardless of the project.
> > The python version of the test is also missing.
> > However, we still need a 22-*.py test so we don't break the regression
> > test.
>
> I though about it, but I considered this new API only touches C API.
> Should the missing test be a dummy? It's no use to have two exactly the
> same tests.
See my other emails on this and the patches I posted yesterday.
> >> { struct scmp_arg_cmp filters[] = ...; ... }
> >
> > Hmmm, I'm not really a fan of this approach using braces to create a
> > new scope.
>
> Do you know a better method? This:
> 1. Allows us to use the same name "filters" for all cases;
> 2. Explicitly specifies that "filters" is only used here, not somewhere
> below;
> 3. Tells no-more-needed array not to linger in stack (if compiler not
> optimizing it);
I don't think there is one magic method which will work in all cases, it
really depends on what you are trying to do and what the existing code looks
like. There are cases where I would recommend creating a new scope with
braces.
In the test case I just went ahead and created a single element array and then
set the value before each call. Check the patches I posted yesterday.
> P.S.
> There is "masked equality" comparator.
> I tried (naively) to add "masked inequality", but failed. Is it tricky
> to do by design?
Adding a new operator is going to require playing with the BPF generator
(gen_bpf.c) and possibly the DB code (db.c); both are very tricky and fragile
bits of code (although I believe they are getting less fragile as time goes
on). You'll likely want to be very careful making your changes and be sure to
run the regression tests often during development.
> Do you consider making comparators more powerful in future?
Yes. There have been a number of suggestions on how to improve the argument
comparisons but I've been focused more on the multi-arch support lately and
haven't had much time to spend on that bit of code yet.
--
paul moore
security and virtualization @ redhat
------------------------------------------------------------------------------
Own the Future-Intel® Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game
on Steam. $5K grand prize plus 10 genre and skill prizes.
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
_______________________________________________
libseccomp-discuss mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libseccomp-discuss
