Hi Denys,
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.
> > and exiting the program isn't necessarily the
> > correct course of action.
>
> Let's see.
> If we opened the file, read it, and then failed to unlink it,
> it means that filesystem is read-only, there is a permission problem
> (e.g. SELinux), or there is an I/O error.
> All of these cases mean that the system is not in expected state,
> and further efforts to "continue as normal" (e.g. try to rewrite
> /var/lib/seedrng/seed.credit) do not seem likely to be productive to me.
> I think it's better to just complain and exit.
No, this isn't okay. It makes it impossible to distinguish which of
the seeds was used by the caller of the program from inspecting the
error code, and it means that it can't attempt the second one if the
first fails (because of, say, the immutable file attribute being set).
I'm not going to make that change.
Jason
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
