2002-07-19 16:28:56

by Joseph Malicki

[permalink] [raw]
Subject: Re: close return value

Those mistakes are your ignorance. The manpage is wrong. It does return -1
on error.
Also, errno is in libc, not the kernel. Man library functions do in fact
use errno.

And it's not an issue of whether an error is "impossible". It's whether or
not you would do
anything if it failed. It's not totally uncommon to actually not care
whether or not it succeeds, but a valiant attempt is enough, such as in the
case of printf.

Sure, if you require an event to be successful to continue you should always
check it. And yes, it's nice to print an error message on close sometimes,
if something is critical. But the question to ask is what you would
actually _DO_ about an error... if the answer is nothing,
then why check it?

-joe


----- Original Message -----
From: "Patrick J. LoPresti" <[email protected]>
To: "Albert D. Cahalan" <[email protected]>
Cc: <[email protected]>
Sent: Friday, July 19, 2002 12:12 PM
Subject: Re: close return value


> "Albert D. Cahalan" <[email protected]> writes:
>
> > You check printf() and fprintf() then? Like this?
> >
> > ///////////////////////////////////////////
> > void err_print(int err){
> > const char *msg;
> > int rc;
> >
> > msg = strerror(err);
> > if(!msg) err_print(errno);
> >
> > do{
> > rc = fprintf(stderr,"Problem: %s\n",msg);
> > }while(rc<0 && errno==EINTR);
> > if(rc<0) err_print(errno);
> > }
> > ///////////////////////////////////////////
>
> Wow, I hardly know where to begin.
>
> I could point out that, at least according to my man page, fprintf()
> returns the number of characters printed; it tells you nothing about
> errors. Also, fprintf() is a library funciton, not a system call, so
> you cannot expect it to put anything meaningful in errno. (I am not
> sure whether these mistakes were part of your sarcasm or your
> ignorance.)
>
> Or I could ask, what part of "assertion failure" did you not
> understand? Yes, the code above is idiotic. But checking that
> fprintf() did not return zero, and calling abort() otherwise, is often
> the right thing to do.
>
> Yes, I exaggerated. There are times when you can reasonably skip
> checking a system call for errors; namely, when you have coded
> defensively enough that any error can do no harm. If you can show
> that the rest of your program operates correctly whether the call
> succeeded or not, then you can skip the error check.
>
> But my main point still holds: You should *not* skip error checks
> because you "know" that the error is "impossible". It takes little
> experience with real-world systems to learn that the "impossible"
> happens with alarming frequency. And when it does, aborting
> immediately is much better than proceeding, because your subsequent
> code is unpredictable and therefore dangerous when your assumptions
> have been violated.
>
> Once you have taken the hit of making a system call, the additional
> cost of checking the return value is irrelevant. So do yourself and
> your users a favor and add the checks.
>
> > Get off your high horse.
>
> Actually, I would rather give others a lift to join me. The view is
> pretty good from up here.
>
> - Pat
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>


2002-07-19 18:45:42

by Patrick J. LoPresti

[permalink] [raw]
Subject: Re: close return value

"Joseph Malicki" <[email protected]> writes:

> Those mistakes are your ignorance. The manpage is wrong. It does
> return -1 on error. Also, errno is in libc, not the kernel. Man
> library functions do in fact use errno.

Sigh. OK, so I should have read SuSv2 instead of my local man page.
Mea culpa. (Once upon a time, the buffered I/O libc routines made no
promises about which system calls they made or when. On such systems,
errno after printf() had no guaranteed semantics.)

> And it's not an issue of whether an error is "impossible". It's
> whether or not you would do anything if it failed. It's not totally
> uncommon to actually not care whether or not it succeeds, but a
> valiant attempt is enough, such as in the case of printf.

If it is a diagnostic printf() to the screen, sure. But an fprintf()
to update some state file on disk is a different matter entirely.

> Sure, if you require an event to be successful to continue you
> should always check it. And yes, it's nice to print an error
> message on close sometimes, if something is critical. But the
> question to ask is what you would actually _DO_ about an error... if
> the answer is nothing, then why check it?

To abort, plain and simple. As I said, if you really think your call
to close() or gettimeofday() or whatever can never fail, you are much
better off dying immediately than proceeding on the assumption that it
succeeded.

Of course, checking errors in order to handle them sanely is a good
thing. Nobody is arguing that. What I am arguing is that failing to
check errors when they can "never happen" is wrong.

Anyway, back to lurker mode for me.

- Pat

2002-07-19 19:22:49

by Lars Marowsky-Bree

[permalink] [raw]
Subject: Re: close return value

On 2002-07-19T14:48:44,
"Patrick J. LoPresti" <[email protected]> said:

> Of course, checking errors in order to handle them sanely is a good
> thing. Nobody is arguing that. What I am arguing is that failing to
> check errors when they can "never happen" is wrong.

Actually, checking for _all_ even remotely possible and checkable error
conditions (if the check doesn't incur an intolerable overhead) is a very very
important requirement for writing high quality code; even if it isn't "fault
tolerant" (because it may not know how to recover, as with the ill-defined
semantics of close() returning error), it will at least be "fail-fast"; giving
an error message close to the cause and terminate in a co-ordinated manner
before corrupting data.

It troubles me deeply that some people hacking on the Linux kernel do not
consider this a good thing.

And with that, I conclude my point and step out of the discussion for good.


Sincerely,
Lars Marowsky-Br?e <[email protected]>

--
Immortality is an adequate definition of high availability for me.
--- Gregory F. Pfister

2002-07-19 19:28:03

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: close return value

Em Fri, Jul 19, 2002 at 09:25:24PM +0200, Lars Marowsky-Bree escreveu:
> On 2002-07-19T14:48:44,
> "Patrick J. LoPresti" <[email protected]> said:
>
> > Of course, checking errors in order to handle them sanely is a good
> > thing. Nobody is arguing that. What I am arguing is that failing to
> > check errors when they can "never happen" is wrong.
>
> Actually, checking for _all_ even remotely possible and checkable error
> conditions (if the check doesn't incur an intolerable overhead) is a very very
> important requirement for writing high quality code; even if it isn't "fault

If the function is not to be checked for errors, lets make it return void and
be done with it. There are few _exceptions_, but one has to understand the
meaning of that word 8)

- Arnaldo

2002-07-20 14:39:23

by Andries Brouwer

[permalink] [raw]
Subject: Re: close return value

On Fri, Jul 19, 2002 at 12:24:33PM -0400, Joseph Malicki wrote:

> Those mistakes are your ignorance. The manpage is wrong.
> It does return -1 on error.

Yes, you are right (or, at least, "a negative value").
Now you deserve a beating for noting that there is a bug on
a man page without submitting a correction, or at least
telling the maintainer. (Yes, that's me.)

> Sure, if you require an event to be successful to continue you should always
> check it. And yes, it's nice to print an error message on close sometimes,
> if something is critical. But the question to ask is what you would
> actually _DO_ about an error... if the answer is nothing,
> then why check it?

But here you are wrong. Even if the program doesn't know what to do,
the user will want to know about it. If I make a backup and some error
occurs then I would be very unhappy if the program were silent about it.

Andries
[email protected]