[Nut-upsdev] Re: [Nut-upsuser] Problem after reboot

[Peter Selinger]

Arjen de Korte wrote:
> 
> I'm taking this thread to nut-devel, since it really doesn't belong in
> nut-users anymore.
> 
> > I propose to rename the function fatal() as fatal_with_errno(), and
> > similarly upslog() as upslog_with_errno(). Reason: this is
> > self-documenting, and will help to avoid accidentally calling fatal()
> > in a situation where errno is not set. fatalx() and upslogx() would
> > remain as before.
> 
> I'm not sure whether the call to fatal() was an accident here (and should
> have been fatalx() instead) or intentional. Assuming it was an accident,
> we might have solved this also by just changing the calls to fatal() into
> fatalx(), without modifying get_user_pwent() at all. Short of this, your
> suggestion fails to address the case for upsdebug() and upsdebugx(), where
> a similar situation exists.

upsdebug() is not currently called anywhere, so I did not include it
in my post, but yes, I was intending to change its name also.

> Changing these functions in all drivers (and documentation) would mean a
> massive amount of changes (although this could be automated) to maintain
> consistency.

I have already done this in my working copy, so at this point, the
required effort is nil.

> In that case, I would also like to see fatalx(), upslogx()
> and upsdebugx() changed to fatal_without_errno(), upslog_without_errno()
> and upsdebug_without_errno(). Personally, I don't think changing all these
> names (and additional typing needed for new ones) is worth the effort and
> we should mainly focus on preventing the fatal() function to print
> obviously wrong information.

Indeed, our posts have crossed, and I also agree with your suggestions
of imporiving the quality of the error messages, and letting fatal()
have a more useful default behavior. These changes would be useful,
but are orthogonal to the renaming issue; both together would make
sense.

The reason I like fatal_with_errno() is because it is
self-documenting; if someone has not read developers.txt recently,
they might not remember that there is a difference between fatal() and
fatalx(). Typing fatal_with_errno() will serve as a useful reminder.
There is no similar benefit to typing fatal_without_errno(), so I had
not proposed to change it for that reason. Ideally the function
currently called fatalx() should just be called fatal(), since the
"without errno" is what one would assume to be the default behavior
anyway. However, renaming it to fatal() would lead to too much
confusion, since a function by that name still exists in older
versions and branches.

> > Any objections?
> 
> It seems that our posts crossed. I just proposed to keep the names as they
> are and just let fatal() behave as fatalx() when errno is zero, to prevent
> printing "success" where the action taken (aborting the program) clearly
> means something is very wrong. As far as I'm concerned, that should be
> enough. 

This is clearly a sensible thing to do, so that users don't get
confused if programmers screwed up. However, it is only a damage
control measure in case fatal() is called improperly. There is no
guarantee that errno==0 if fatal() is called after some failure that
did not set errno. In that situation, errno might well be non-0, because some previous (non-fatal) system call set it. 

> A different effort (and more important one) would be to be
> somewhat more verbose in telling the user what's wrong before bailing out
> with a call to fatal() or fatalx(). Displaying/logging 'getpwnam(nut)'
> just isn't very informative, even without calling it a success.

Agreed. -- Peter