> > For example, in read_new_seed() we probably don't need the errno=EIO,
> > for the user it should be sufficient to
> > bb_simple_error_msg("unable to read new seed"); i suppose?  
> 
> Not a huge fan of that, but okay, sure. bb_simple_perror_msg should
> suppress errno=0 "Success" messages anyway, so not the worst thing.
> Saves 18 bytes.

thanks
> 
> >
> > In v5, seed_from_file_if_exists i personally think that the errno
> > handling does not improve readability TBH. But that's just me.  
> 
> Also not a huge fan of removing that, but nonetheless I'll cut back
> the errno handling entirely there. Should be functionally the same.
> Saves an additional 18 bytes.

thanks
> 
> > But either way, if seed_len==0 why you don't unlink the file?  
> 
> I think you misread the function. For seed_len==0 I do unlink the

Of course you're right.
+ if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) {
is what you wrote, and the seed_len != 0 cannot be juggled around by a
compiler.

> > In this case the file would be garbage anyway, no? Or, the other way
> > round:
> > Why would the file exist and be readable but have no data, IIUC?  
> 
> Filesystem corruption for battery powered devices, package managers
> and templates that "touch" a file, etc. More common than you might
> think. In that case, the right course of action is to unlink the file
> and then act as though it didn't exist.

Ok.

> > In main, i'd have used xopen_xwrite_close if a single error-code is
> > enough or needed at all.  
> 
> No, again, not acceptable, since the fd must be fsync()ed before
> renaming it to become creditable.

well that'd be xopen_xwrite_close(), fsync and then rename_or_warn but
since for fsync we'd need an fd there might no (big?) net benefit.
Pity, i'd have estimated a 2k for the applet and we're just a tad over
that. :)
> 
> > And we have rename_or_warn() if you can live without exit(64) in this
> > case.  
> 
> This doesn't reduce code size.

btw for the getuid() failure, we have bb_msg_you_must_be_root[] or
bb_msg_perm_denied_are_you_root[]

So what about the errno in seed_rng.
Can we just leave errno alone and report a failure in this function
only? And only afterwards return -1 and remove the diagnostics from the
caller?

And just to make that explicit: What's the deal with all those bits in
the exit code. Are these really necessary?

I forgot, isn't O_RDONLY sufficient for ioctl? And IIRC there was even
a non-io mode to just obtain the fd, linux specific, wasn't there.
Doesn't save anything of course, but RDONLY looks cleaner..

thanks for your patience,
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to