[Toybox] [CLEANUP] tftpd

Rob Landley rob at landley.net
Mon Dec 23 04:46:02 PST 2013


On 11/24/2013 08:17:24 PM, ibid.ag at gmail.com wrote:
> I ran into build errors with tftpd and dumpleases, so here's a fix:
> POLL_IN is a POSIX macro in signal.h, so we need to use another label.

Which libc hit that?

> toynet.h was deleted before dumpleases was merged.
> While I was at it, I thought I'd shut up the warnings in blkid by
> explicitly naming the types of three numbers.

Already removed toynet.h from dumpleases. And you didn't mention the  
blkid stuff in the patch, but I already noticed that too.

My tftpd build error is that change_identity isn't declared? Huh. Ah,  
it's in login.c and presumably needs to be moved to lib/pending.c or  
similar. Hmmm, it's only 3 lines, I can just review it now...

I think change_identity() should be an xfunction() that exits on error,  
for two reasons. 1) Both existing users do that already, 2) How do you  
recover from a partial failure? If you change uid but not gid, do you  
still have access to retry after that?

xchange_identity() is a sort of awkward name (none of the other  
xnames() have an underscore), maybe xsetidentity()? Hmmm, it would be  
nice to put the argument time (passwd) in the name but we're not  
actually setting anybody's password. And xsetuidgid() means having to  
remember which order uid and gid go in... Maybe xsetuser()?

There's already a xsetuid() in xwrap.c, possibly its users should be  
replaced by this? Only two users, one in su.c which has a struct passwd  
and one in main.c which doesn't. Replace the su.c one and have the  
main.c one check its own error.

Other in-passing tftpd.c cleanups:

I put a space between argument globals and non-argument globals.
TT starts zeroed, don't need to re-zero entries in it.
STDIN_FILENO has been 0 since 1969, even DOS copied that. Just say 0.
Added an xchroot() using xchdir() to lib/xwrap.c.

Why is this calling endgrent() if it never called getgrent()? Is this  
due to initgroups()? If so, why didn't login.c call it, and should it  
be in xsetuser()? (Removing it for now, if somebody could explain why  
it's needed I'd appreciate it...)

Right, more tftpd cleanup later. In the meantime, I think I've covered  
the issues raised in this email.

Thanks,

Rob


More information about the Toybox mailing list