[dmidecode] [PATCH 1/3] util: Don't close the same file descriptor twice

[Jean Delvare]

If myread() fails, it closes the file descriptor it was reading from.
On success, it does not. The caller is always closing the file
descriptor afterward, which results in double close in the error
case. This causes an additional error:

/dev/mem: Bad file descriptor

As the caller is always taking care of closing the file descriptor,
there is no need for myread() to do it.

Signed-off-by: Jean Delvare <***@suse.de>
Fixes: 458f73d58c24 ("Fix error paths in mem_chunk")
---
util.c | 2 --
1 file changed, 2 deletions(-)

--- dmidecode.orig/util.c 2018-08-01 10:27:05.874390642 +0200
+++ dmidecode/util.c 2018-08-01 10:27:15.950517651 +0200
@@ -59,7 +59,6 @@ static int myread(int fd, u8 *buf, size_
{
if (errno != EINTR)
{
- close(fd);
perror(prefix);
return -1;
}
@@ -70,7 +69,6 @@ static int myread(int fd, u8 *buf, size_

if (r2 != count)
{
- close(fd);
fprintf(stderr, "%s: Unexpected end of file\n", prefix);
return -1;
}

[Jean Delvare]

Now that we check the file size before reading, we no longer expect
to hit the end of file prematurely. This means we can call myread()
instead of reimplementing it, thus removing some code redundancy.

Signed-off-by: Jean Delvare <***@suse.de>
---
util.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)

--- dmidecode.orig/util.c 2018-08-01 10:40:07.043544869 +0200
+++ dmidecode/util.c 2018-08-01 10:47:29.231481728 +0200
@@ -99,8 +99,6 @@ void *read_file(off_t base, size_t *max_
{
struct stat statbuf;
int fd;
- size_t r2 = 0;
- ssize_t r;
u8 *p;

/*
@@ -137,25 +135,12 @@ void *read_file(off_t base, size_t *max_
goto out;
}

- do
- {
- r = read(fd, p + r2, *max_len - r2);
- if (r == -1)
- {
- if (errno != EINTR)
- {
- perror(filename);
- free(p);
- p = NULL;
- goto out;
- }
- }
- else
- r2 += r;
- }
- while (r != 0);
+ if (myread(fd, p, *max_len, filename) == 0)
+ goto out;
+
+ free(p);
+ p = NULL;

- *max_len = r2;
out:
close(fd);

[Jean Delvare]

Make the code flow of read_file() as similar as possible to
mem_chunk() to make it easier to read and to avoid bugs:
* Move the call to lseek() right before reading the data. This
even saves one instruction.
* Check for error on close(). This is good practice anyway.

Signed-off-by: Jean Delvare <***@suse.de>
---
util.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

--- dmidecode.orig/util.c 2018-08-01 10:55:18.678591293 +0200
+++ dmidecode/util.c 2018-08-01 11:00:03.589815035 +0200
@@ -112,14 +112,6 @@ void *read_file(off_t base, size_t *max_
return NULL;
}

- if (lseek(fd, base, SEEK_SET) == -1)
- {
- fprintf(stderr, "%s: ", filename);
- perror("lseek");
- p = NULL;
- goto out;
- }
-
/*
* Check file size, don't allocate more than can be read.
*/
@@ -135,14 +127,23 @@ void *read_file(off_t base, size_t *max_
goto out;
}

+ if (lseek(fd, base, SEEK_SET) == -1)
+ {
+ fprintf(stderr, "%s: ", filename);
+ perror("lseek");
+ goto err_free;
+ }
+
if (myread(fd, p, *max_len, filename) == 0)
goto out;

+err_free:
free(p);
p = NULL;

out:
- close(fd);
+ if (close(fd) == -1)
+ perror(filename);

return p;
}