Date: Thu, 16 Jul 2020 19:49:56 +0300 From: "Dmitry V. Levin" <ldv@...linux.org> To: owl-dev@...ts.openwall.com Subject: Re: [PATCH 0/5] pam_tcb update On Thu, Jul 16, 2020 at 06:30:22PM +0200, Solar Designer wrote: > 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? The current edition of readdir_r(3) manpage at https://man7.org/linux/man-pages/man3/readdir_r.3.html says: "It is recommended that applications use readdir(3) instead of readdir_r()." It also gives a rationale for this move. > > 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. OK > > 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? Yes, this is the only remaining piece of NIS+ support, so we can safely drop it instead of trying to come up with a fix. -- ldv
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.