Hi Jan, On Fri, Sep 15, 2017 at 07:23:31AM -0400, Jan Cerny wrote: > Hi, > > I have reviewed your patch. I have tested it on my Ubuntu Server 16.04.3, > and it works great. > > The patch overall looks good, but I have a few minor findings: > > src/OVAL/probes/unix/linux/apparmorstatus.c : > * It shouldn't fail hard if you don't run it as root. In general OpenSCAP > doesn't terminate > just because it couldn't get some data. I suggest `return 0` on line 362. > * Please use `snprintf` instead of `sprintf`. It's safer. > * On line 320, SEXP_free(item) shouldn't be commented out. But please make > sure > everything is freed properly then. > * Please don't mix spaces and tabs. Tabs should be used to indent everywhere > in C code. > * On line 200 you close a file that was already closed on line 177. > * I guess that the sizeof operator on line 273 should take "struct > aa_profile". > * Compiler reports some warnings: > ``` > unix/linux/apparmorstatus.c: In function ‘probe_main’: > unix/linux/apparmorstatus.c:330:24: warning: variable ‘over’ set but not used > [-Wunused-but-set-variable] > oval_schema_version_t over; > ^ > unix/linux/apparmorstatus.c: In function ‘aa_isenabled’: > unix/linux/apparmorstatus.c:90:2: warning: ignoring return value of ‘read’, > declared with attribute warn_unused_result [-Wunused-result] > (void) read(fd, &status, 1); > ``` >
Thanks for the review. I'll fix them asap. Problem though is I'm busy atm. I'll look at this monday, hoping to fix the major issues. Cheers, -- Bruno Ducrot -- Which is worse: ignorance or apathy? -- Don't know. Don't care. _______________________________________________ Open-scap-list mailing list Open-scap-list@redhat.com https://www.redhat.com/mailman/listinfo/open-scap-list
