Date: Thu, 16 Jul 2020 18:30:22 +0200 From: Solar Designer <solar@...nwall.com> To: owl-dev@...ts.openwall.com Subject: Re: [PATCH 0/5] pam_tcb update On Thu, Jul 16, 2020 at 12:56:07AM +0300, Dmitry V. Levin wrote: > These -Wpointer-sign warnings look harmless. > I suppose they are quite old, though. > I've committed a fix. The warnings are gone for me. Thanks! > I also see the following warnings: > nss.c:111:3: warning: 'readdir_r' is deprecated [-Wdeprecated-declarations] > tcb_unconvert.c:245:2: warning: ignoring return value of 'chown', declared with attribute warn_unused_result [-Wunused-result] > tcb_chkpwd.c:58:4: warning: ignoring return value of 'seteuid', declared with attribute warn_unused_result [-Wunused-result] > tcb_chkpwd.c:61:4: warning: ignoring return value of 'seteuid', declared with attribute warn_unused_result [-Wunused-result] > > We probably should use readdir instead of readdir_r, this would fix the > first warning. Could we? This is inside _nss_tcb_getspent_r(), which is sort of meant to be reentrant - but I don't understand what this really means for that particular function given that it's stateful - each subsequent call is supposed to return the next entry. Maybe what is meant is only that the return value buffer is caller-provided, so if different buffers are used then the previous ones are not overwritten by subsequent calls. Not that the function is really reentrant. Do you know? Also, why is readdir_r() deprecated? Is readdir() now reentrant? > There is not much we could do about -Wunused-result warnings. > In case of 'chown' it make sense to issue a warning. Yes, I think we can call perror() just like we do on the previous chown()'s error, but we don't have to return 1 on this one. > In case of first 'seteuid' error we could skip the second 'seteuid' > invocation. Then what to do if the first one succeeds, but the second one fails? Luckily, this is a separate program, which at that point is about to terminate. Also, this whole piece is for NIS (right?), the support for which we've just dropped from the rest of tcb. Should we possibly drop this piece, too? Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.