Hi Denys, On Sat, Apr 30, 2022 at 4:09 AM Denys Vlasenko <vda.li...@googlemail.com> wrote: > > On Fri, Apr 29, 2022 at 6:57 PM Jason A. Donenfeld <ja...@zx2c4.com> wrote: > > On Fri, Apr 29, 2022 at 6:04 PM Denys Vlasenko <vda.li...@googlemail.com> > > wrote: > > > On Wed, Apr 27, 2022 at 6:55 PM Jason A. Donenfeld <ja...@zx2c4.com> > > > wrote: > > > > On Wed, Apr 27, 2022 at 06:15:50PM +0200, Denys Vlasenko wrote: > > > > > if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) { > > > > > bb_perror_msg("can't%s seed", " remove"); > > > > > return -1; > > > > > } > > > > > > > > > > Why can't the entire above if() be replaced with xunlink(filename) ? > > > > > > > > It cannot be replaced with that. The fsync() must happen before other > > > > operations move forward, > > > > > > Why? It should be explained in a comment, and explained well enough > > > so that future developers do not repeatedly try to remove it > > > because "I don't see why it's necessary". > > > > I'll add a comment and submit v2 of my recent patch. > > It'll be faster if we just discuss it in this thread.
Surely not, since I submitted v2 a day prior to your reply. It looks like you responded over there, so I'll follow up in that thread. > > > > and exiting the program isn't necessarily the > > > > correct course of action. > > Of course. I'm not saying that "immediately exit with error message" > _always_ is the correct approach. > > What I saw a lot over the years is that error paths > tend to be not well-tested, and nevertheless, even when they > are not buggy per se, the error conditions they are reacting to > are often a part of a bigger problem (usually beyond the ability > of the code in question to detect, much less sensibly correct). > > For example, if you get a ENOSPC on write, you might be > tempted to return a special exit code signifying this specific error. > > However. > I'd bet you real money that NOT ONE of callers of your tool > will ever bother checking specifically for this exit code. > At best, the callers will react uniformly for any non-zero exit code as > "something went wrong". Quite often, check for !0 exit code will be > entirely forgotten. I'm not going to bet you real money on anything, as this is volunteer work, unpaid, etc. Please don't offer that again. Anyway, your point is rubbish. The promise of SeedRNG is that it tries to always do the maximal amount of useful things, and safely handles the various error cases that can happen, because the kernel's interface is complicated and a modicum of errors can transpire. The user can then decide what to do with those errors -- halt the system boot under certain circumstances, try to pull from some expensive last ditch effort, send an alert email to an administrator, etc. It's both important that the user is notified about this via the error code AND that SeedRNG continues on doing the maximum amount of initialization that it can do safely (i.e. without introducing a vulnerability). Please stop attempting to make SeedRNG as worthless as all the previous other attempts to do RNG initialization. Your direction toward fragmentation, non-robustness, shortsighted thinking, and so forth is only going to perpetuate a problem that has long plagued the Linux ecosystem. Stop doing that. Jason _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox