On Tue, Sep 6, 2016 at 1:22 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > On 09/06/2016 04:06 PM, William Roberts wrote: >> On Sep 6, 2016 13:01, "Stephen Smalley" <sds@xxxxxxxxxxxxx >> <mailto:sds@xxxxxxxxxxxxx>> wrote: >>> >>> On 09/06/2016 11:51 AM, william.c.roberts@xxxxxxxxx >> <mailto:william.c.roberts@xxxxxxxxx> wrote: >>> > From: William Roberts <william.c.roberts@xxxxxxxxx >> <mailto:william.c.roberts@xxxxxxxxx>> >>> > >>> > The current process_file() code will open the file >>> > twice on the case of a binary file, correct this. >>> > >>> > The general flow through process_file() was a bit >>> > difficult to read, streamline the routine to be >>> > more readable. >>> > >>> > Detailed statistics of before and after: >>> > >>> > Source lines of code reported by cloc on modified files: >>> > before: 1090 >>> > after: 1102 >>> > >>> > Object size difference: >>> > before: 195530 bytes >>> > after: 195563 bytes >>> >>> Old logic would use the .bin file as long as it is not older than the >>> base file; new logic will only use the .bin file if it is newer. The >>> end result on my system was that it was using the text file instead. >> >> I'm not following here. I spent a lot of time with strace comparing old >> vs new behavior here. If x and x.bin exist, it should use x.bin if it's >> newer or the same age as x? Is that correct? > > Yes. strace before shows that it opens the text file and the bin file, > and then proceeds to map the bin file and close the text file without > processing it. strace after shows that it stats them both and only > opens the text file. The st_mtime values are the same (possibly we > ought to be comparing the full st_mtim with nanosecond precision > instead, but regardless, if equal, we ought to use the bin file) because > we now generate the bin file in the sandbox and then copy them both to > /etc/selinux at the same time (commit > a7334eb0de98af11ec38b6263536fa01bc2a606c). You wouldn't see that if > your policy was last built/updated with the older selinux userspace > prior to that commit; if you run semodule -B after installing the > current userspace, you will get that behavior. Ok this should be simple enough the > should go to >= for the mtime comparison. We can change the granularity in a separate patch, if you'd like. > >> >>> >>> Also, there are some memory leaks in there; run it under valgrind, e.g. >>> valgrind --leak-check=full matchpathcon /etc >> >> OK I'll run that test. I cant reproduce: <snip> _______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.