Hi Kees, On 10 March 2015 at 18:41, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > On Tue, Mar 10, 2015 at 10:24 AM, Michael Kerrisk (man-pages) > <mtk.manpages@xxxxxxxxx> wrote: >> Hi Kees, >> >> I was looking further at the seccomp(2) man page we put togetehr a >> while ago, and in particular at the example [1]. I realized that from >> a readability view, the order of the statements is rather contorted, >> so that we have branches to the end of the code (labels 5 and 6) to >> handle the syscall and arch mismatch cases, rather than swapping the >> branch order on the statements labeled 0 and 1, and having the kill >> statements immediately follow the test statements. >> >> However, I presume the ordering is as it is for efficiency reasons, so >> that the common case involves no branch forward (i.e., a branch offset >> of 0). Is that the case? If so, probably something should be said in >> the man page. > > I've never attempting timing tests to see if branching in the common > path is actually any more expensive, but it's not unreasonable. I Okay. > think the main reason it was written this way was actually for size. > There's 1 kill statement, which avoids repeating it, and (I think) is > more readable. FWIW, I find it less readable ;-). That's because one has to count the BPF statements to branch over (that's why I added the "[n]" labels to the code). Also, it makes the code trickier to maintain, since, if one adds some code in the branched over region, then the branch offset needs to be modified. Just my 2¢. > Ultimately, it's really up to the author of the filter > to do what they want, but there wasn't a specific speed efficiency > goal with how it was written. Okay -- thanks for clarifying. Cheers, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/ -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
