On 01/28/2013 10:44 AM, Jakub Hrozek wrote:
On Mon, Jan 28, 2013 at 10:37:19AM +0100, Pavel Březina wrote:
We should also test the return value of close() on the succeeding
lines. I wonder why Coverity didn't argue here.
My guess is that we also don't check close() return value elsewhere. The
way I've seen Coverity work is that if most other occurences are
checked, then Coverity raises a flag is one is not.
Yes I though so too. That is also the reason, why I didn't check the
return value of close here. If we want to check it, we must change it
everywhere where close() is used (Otherwise Coverity will probably
complain). Do we want to check close() return values? If so I can post
another patch related to close() only that will check it everywhere, but
I think it is not necessary.
New patch is attached.
Thanks
Michal
>From d9087cf7dc40c9327138494d92b905927e9f4f27 Mon Sep 17 00:00:00 2001
From: Michal Zidek <[email protected]>
Date: Fri, 25 Jan 2013 16:41:07 +0100
Subject: [PATCH] Unchecked return value in files.c
Found by coverity.
https://fedorahosted.org/sssd/ticket/1791
---
src/tools/files.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/tools/files.c b/src/tools/files.c
index 4a6625e..c5dbb71 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -583,6 +583,7 @@ copy_dir(struct copy_ctx *cctx,
const struct stat *src_dir_stat)
{
errno_t ret;
+ errno_t dret;
int dest_dir_fd = -1;
DIR *dir = NULL;
struct dirent *ent;
@@ -675,7 +676,14 @@ copy_dir(struct copy_ctx *cctx,
ret = EOK;
done:
- if (dir) closedir(dir);
+ if (dir) {
+ dret = closedir(dir);
+ if (dret != 0) {
+ dret = errno;
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Failed to close directory: %s.\n", strerror(dret)));
+ }
+ }
if (dest_dir_fd != -1) {
close(dest_dir_fd);
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel