Daniel Golle <[email protected]> wrote: > On Mon, Jan 04, 2021 at 05:08:22PM -0000, Karl Palsson wrote: > > > > Daniel Golle <[email protected]> wrote: > > > On Sat, Jan 02, 2021 at 10:01:36PM +0800, [email protected] > > > wrote: > > > > > > - chown(p->pw_dir, p->pw_uid, p->pw_gid); > > > > + if (chown(p->pw_dir, p->pw_uid, p->pw_gid)) > > > > + fprintf(stderr, "Failed to change > > > > ownership for %s\n", p->pw_dir); > > > > > > Please let's not have a custom error message for cases which > > > practically never occur. If we would really cover all that, > > > around 80% of the size of executables like procd would be error > > > messages. Imho an assertion is the right thing to do here. > > > > > > > Do we compile with assertions enabled? > > Interesting point actually: I just checked, assertions are > compiled into code unless NDEBUG is defined which we don't. > Could be an idea for tiny targets to do so...
That... might make things worse :) For instance, in the patch as
it is proposed now...
- chown(p->pw_dir, p->pw_uid, p->pw_gid);
+ assert(chown(p->pw_dir, p->pw_uid, p->pw_gid) == 0);
the chown will never get called if you compile with NDEBUG....
eg, (We want to get 2 printed in every case, this is when the
asserts pass)
karlp@beros:~/src/junk$ cat hate.c
#include <assert.h>
#include <stdio.h>
static int k;
void call_it_safe(void) {
assert(++k > 0);
}
void yolo(void) {
k++;
}
int main() {
call_it_safe();
yolo();
printf("y0: %d\n", k);
}
karlp@beros:~/src/junk$ cc hate.c -DNDEBUG -o hate_ndebug
karlp@beros:~/src/junk$ cc hate.c -o hate karlp@beros:~/src/junk$
./hate y0: 2 << good karlp@beros:~/src/junk$ ./hate_ndebug y0: 1
<<< oops! karlp@beros:~/src/junk$
This is akin to the classic problem of logging macros turning off
code. You need to introduce an extra variable for the assert to
check. (If you want to keep going down this path)
Sincerely,
Karl Palsson
OpenPGP-digital-signature.html
Description: OpenPGP Digital Signature
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
