block/file-posix.c | 1 + 1 file changed, 1 insertion(+)
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/file-posix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index c4c0663..e6170f4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st)
}
out:
+ close(fd);
g_free(sysfspath);
return ret;
#else
--
2.9.3
Am 14.03.2017 um 16:43 hat Fam Zheng geschrieben: > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/file-posix.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/file-posix.c b/block/file-posix.c > index c4c0663..e6170f4 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) > } > > out: > + close(fd); Should we make this conditional on fd != -1? > g_free(sysfspath); > return ret; > #else Kevin
On Tue, 03/14 16:50, Kevin Wolf wrote: > Am 14.03.2017 um 16:43 hat Fam Zheng geschrieben: > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block/file-posix.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/block/file-posix.c b/block/file-posix.c > > index c4c0663..e6170f4 100644 > > --- a/block/file-posix.c > > +++ b/block/file-posix.c > > @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) > > } > > > > out: > > + close(fd); > > Should we make this conditional on fd != -1? OK, that is cleaner. Fam > > > g_free(sysfspath); > > return ret; > > #else > > Kevin
On 03/14/2017 10:43 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/file-posix.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/file-posix.c b/block/file-posix.c > index c4c0663..e6170f4 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) hdev_get_max_segments() is not part of master yet; should this just be treated as a fixup! to a pending patch? > } > > out: > + close(fd); > g_free(sysfspath); > return ret; > #else > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/14/2017 11:11 AM, Eric Blake wrote: > On 03/14/2017 10:43 AM, Fam Zheng wrote: >> Signed-off-by: Fam Zheng <famz@redhat.com> >> --- >> block/file-posix.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/block/file-posix.c b/block/file-posix.c >> index c4c0663..e6170f4 100644 >> --- a/block/file-posix.c >> +++ b/block/file-posix.c >> @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) > > hdev_get_max_segments() is not part of master yet; should this just be > treated as a fixup! to a pending patch? Scratch that - I forgot to 'git pull' on my end. Commit message could usefully be improved by mentioning commit 9103f1ce as the spot the problem was introduced. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/14/2017 11:14 AM, Eric Blake wrote: > On 03/14/2017 11:11 AM, Eric Blake wrote: >> On 03/14/2017 10:43 AM, Fam Zheng wrote: >>> Signed-off-by: Fam Zheng <famz@redhat.com> >>> --- >>> block/file-posix.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/block/file-posix.c b/block/file-posix.c >>> index c4c0663..e6170f4 100644 >>> --- a/block/file-posix.c >>> +++ b/block/file-posix.c >>> @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) >> >> hdev_get_max_segments() is not part of master yet; should this just be >> treated as a fixup! to a pending patch? > > Scratch that - I forgot to 'git pull' on my end. > > Commit message could usefully be improved by mentioning commit 9103f1ce > as the spot the problem was introduced. > > Reviewed-by: Eric Blake <eblake@redhat.com> > Question: does valgrind or any other tool complain about: + if (fd == -1) { + ret = -errno; + goto out; + } ... +out: + close(fd); + g_free(sysfspath); + return ret; this being a useless syscall to close(-1) ? If so, you need to wrap in an 'if (fd != -1)' -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Am 14.03.2017 um 17:11 hat Eric Blake geschrieben: > On 03/14/2017 10:43 AM, Fam Zheng wrote: > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > block/file-posix.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/block/file-posix.c b/block/file-posix.c > > index c4c0663..e6170f4 100644 > > --- a/block/file-posix.c > > +++ b/block/file-posix.c > > @@ -703,6 +703,7 @@ static int hdev_get_max_segments(const struct stat *st) > > hdev_get_max_segments() is not part of master yet; should this just be > treated as a fixup! to a pending patch? It is already in master, since yesterday, I think. Kevin
© 2016 - 2024 Red Hat, Inc.