[2016-07-27 21:40] [email protected]
> scrīpsit markus schnalke <[email protected]>:
> | [2016-07-27 11:25] [email protected]
> | >
> | > + if (fstat(0, &s1) == NOTOK) {
> | > + adios(EX_IOERR, "fstat", "unable to read from stdin");
> | > + } else if (!(S_ISREG(s1.st_mode) || S_ISFIFO(s1.st_mode))) {
> | > + adios(EX_USAGE, NULL, "missing input from stdin");
> |
> | Why do you do these two checks? To control the error message?
>
> Exactly. But your subsequent comment (below) has led me to consider
> the second check unnecessary. Do you consider the first check also
> unnecessary?
What conditions could lead to a failure of the first check? The
manpage of fstat(2) says:
EACCES Search permission is denied for one of the directories in the
path prefix of path. (See also path_resolution(7).)
EBADF fd is bad.
EFAULT Bad address.
ELOOP Too many symbolic links encountered while traversing the path.
ENAMETOOLONG
path is too long.
ENOENT A component of path does not exist, or path is an empty string.
ENOMEM Out of memory (i.e., kernel memory).
ENOTDIR
A component of the path prefix of path is not a directory.
EOVERFLOW
path or fd refers to a file whose size, inode number, or number
of blocks cannot be represented in, respectively, the types
off_t, ino_t, or blkcnt_t. This error can occur when, for exam‐
ple, an application compiled on a 32-bit platform without
-D_FILE_OFFSET_BITS=64 calls stat() on a file whose size exceeds
(1<<31)-1 bytes.
Is one of them relevant and could not be detected by actually
reading from the fd? I'd say no, hence, drop the pre-checks and
just try to read.
Anyway, it is uncommon, that one would not be able to read
from stdin, hence a pre-check like in the maildrop-empty
case (which is common) is unnecessary.
But, you know, I haven't borrowed wisdom. So, if someone has a
different opinion or just a thought to consider, please speak
up!
> I've remove the second of the two checks from the patch.
Good, because it would have prevented us from reading a character
device. With your first patch:
inc -file - </dev/null
inc: missing input from stdin
No need to forbid that. Someone might have a cool hardware device
that exports mboxes using a character device. ;-)
(Please add a test with /dev/null to the testcase. Should be equal
to ``printf "" | inc -file -'' ... would be best if you add both
checks. These are extreme cases. It's always good to include them
in the testsuite.)
> | > @@ -480,7 +489,7 @@ giveup:;
> | > else
> | > admonish(newmail, "error zero'ing");
> | > }
> | > - } else if (noisy) {
> | > + } else if (noisy && newmail) {
> | > printf("%s not zero'd\n", newmail);
> | > }
> |
> | ... although it might confuse users I would have liked the humor
> | in: ``- not zero'd'' or ``stdin not zero'd''. ;-)
>
> :)
I'm still undecided if my comment was just for fun, or if would
like to have this message printed. To be fair, the version with
the dash is confusing and setting `newmail' to "stdin" might
have other disadvantages ... Well, maybe it really was just for
the humorous aspect ... :-)
> I've attached to this message also a second patch that updates inc(1)
> to mention that inc can read stdin.
Thanks for that, too. When we've decided about the fstat(2)
check and you provide a third patch, include the testcase in
the patch as well.
meillo
