On 23. nov. 2013 10:53, Steffan Karger wrote:
> Hi David,
>
> This solution looks good. I did not test, but I do have one minor
> comment after glancing at the code:
>
> @@ -2662,7 +2700,14 @@ check_cmd_access(const char *command, const
> char *opt) * only requires X_OK to function on Unix - a scenario
> not unlikely to * be seen on suid binaries. */ - return_code =
> check_file_access(CHKACC_FILE, argv.argv[0], X_OK, opt); + if
> (chroot) + { + return_code =
> check_file_access_chroot(chroot, CHKACC_FILE, argv.argv[0], X_OK,
> opt); + } + else + { + return_code =
> check_file_access(CHKACC_FILE, argv.argv[0], X_OK, opt); + }
>
>
> This if seems redundant here, as chroot is checked by
> check_file_access_chroot itself. Furthermore, for the other
> occurences of check_file_access you stick to just replacing it
> with check_file_acess_chroot. I would suggest to do that here too.Thanks a lot for the review, Steffan! Yeah, you're absolutely right. I have no strong feelings for either approaches. But the initial reason I did it in this explicit way, was to be, well, explicit about that we do something different when chroot is used - instead of burying it one level deeper down. On the other hand, simplicity can also be just the better approach. I hold no grudges ;-) I've mailed a v2 version of this patch implementing it the way you suggest. And, surely, this change is even slightly less intrusive than my first approach. So I'll let it be up to you, Josh and the rest of the ML to decide which approach you prefer more ... and Gert will apply whatever gets an ACK I presume :) David S. -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
