Re: fixes for coverity warnings to cat(1)

2015-07-29 Thread Sevan Janiyan 


On 21/07/2015 00:16, Sevan Janiyan wrote:
 Hi,
 Attached is a diff for a couple of issues raised by coverity, obtained
 from NetBSD src/bin/cat/cat.c r1.53  r1.54
 
From the commit message in NetBSD CVS:
 bin/cat/cat.c 976654 Argument cannot be negative
  (missing check for fileno result, stdout)
 bin/cat/cat.c 976653 Improper use of negative value
  (missing check for fileno result, stdin)
 
 Diff also adds a skip label used by the changes which appeared in NetBSD
 at a different revision in the past.
 
 
 Sevan Janiyan

Just following up on this to check it hadn't been missed due to my bad
timing with the hackathon finishing.



Sevan
From NetBSD
cat.c r1.53 r1.54

Index: bin/cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -r1.21 cat.c
--- bin/cat/cat.c   16 Jan 2015 06:39:28 -  1.21
+++ bin/cat/cat.c   20 Jul 2015 23:08:00 -
@@ -200,15 +200,20 @@
filename = stdin;
do {
if (*argv) {
-   if (!strcmp(*argv, -))
+   if (!strcmp(*argv, -)) {
fd = fileno(stdin);
-   else if ((fd = open(*argv, O_RDONLY, 0))  0) {
+   if (fd  0)
+   goto skip;
+   } else if ((fd = open(*argv, O_RDONLY, 0))  0) {
+skip:
warn(%s, *argv);
rval = 1;
++argv;
continue;
}
filename = *argv++;
+   } else if (fd  0) {
+   err(1, stdin);
}
raw_cat(fd);
if (fd != fileno(stdin))
@@ -226,6 +231,8 @@
struct stat sbuf;
 
wfd = fileno(stdout);
+   if (wfd  0)
+   err(1, stdout);
if (buf == NULL) {
if (fstat(wfd, sbuf))
err(1, stdout);

Re: fixes for coverity warnings to cat(1)

2015-07-29 Thread Todd C. Miller 
Note that fileno() does not set errno so you should probably set
errno to EBADF before the calls to warn() or err().

 - todd

Re: fixes for coverity warnings to cat(1)

2015-07-29 Thread Ted Unangst 
Sevan Janiyan wrote:
 
 
 On 21/07/2015 00:16, Sevan Janiyan wrote:
  Hi,
  Attached is a diff for a couple of issues raised by coverity, obtained
  from NetBSD src/bin/cat/cat.c r1.53  r1.54
  
 From the commit message in NetBSD CVS:
  bin/cat/cat.c 976654 Argument cannot be negative
   (missing check for fileno result, stdout)
  bin/cat/cat.c 976653 Improper use of negative value
   (missing check for fileno result, stdin)
  
  Diff also adds a skip label used by the changes which appeared in NetBSD
  at a different revision in the past.

have you tested this? does it make a difference?

when does fileno(stdin) return -1?

Re: fixes for coverity warnings to cat(1)

2015-07-29 Thread Todd C. Miller 
On Wed, 29 Jul 2015 17:41:19 -0400, Ted Unangst wrote:

 when does fileno(stdin) return -1?

It looks like _file in struct __sFILE (aka FILE) is only -1 for
non-files, e.g. for things like snprintf().  I've verified that
fileno(stdin) still returns 0 even when stdin is closed.  This is
true regardless of whether it is closed via fclose() in the current
process or it inherits a closed fd at exec time.

I don't really see any need for this diff.

 - todd

Re: fixes for coverity warnings to cat(1)

2015-07-29 Thread Sevan Janiyan 


On 29/07/2015 23:24, Todd C. Miller wrote:
 I don't really see any need for this diff.

ugh, sorry if I wasted your time, thanks for the explanation.


Sevan

fixes for coverity warnings to cat(1)

2015-07-20 Thread Sevan Janiyan 
Hi,
Attached is a diff for a couple of issues raised by coverity, obtained
from NetBSD src/bin/cat/cat.c r1.53  r1.54

From the commit message in NetBSD CVS:
bin/cat/cat.c 976654 Argument cannot be negative
 (missing check for fileno result, stdout)
bin/cat/cat.c 976653 Improper use of negative value
 (missing check for fileno result, stdin)

Diff also adds a skip label used by the changes which appeared in NetBSD
at a different revision in the past.


Sevan Janiyan
From NetBSD
cat.c r1.53 r1.54

Index: bin/cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -r1.21 cat.c
--- bin/cat/cat.c   16 Jan 2015 06:39:28 -  1.21
+++ bin/cat/cat.c   20 Jul 2015 23:08:00 -
@@ -200,15 +200,20 @@
filename = stdin;
do {
if (*argv) {
-   if (!strcmp(*argv, -))
+   if (!strcmp(*argv, -)) {
fd = fileno(stdin);
-   else if ((fd = open(*argv, O_RDONLY, 0))  0) {
+   if (fd  0)
+   goto skip;
+   } else if ((fd = open(*argv, O_RDONLY, 0))  0) {
+skip:
warn(%s, *argv);
rval = 1;
++argv;
continue;
}
filename = *argv++;
+   } else if (fd  0) {
+   err(1, stdin);
}
raw_cat(fd);
if (fd != fileno(stdin))
@@ -226,6 +231,8 @@
struct stat sbuf;
 
wfd = fileno(stdout);
+   if (wfd  0)
+   err(1, stdout);
if (buf == NULL) {
if (fstat(wfd, sbuf))
err(1, stdout);