<html><body><p><tt>I remember reviewing a very similar code ( or perhaps the same ) in the past. Is it a new patch now ?</tt><br><br><tt>Here is what I had sent before</tt><br><br><tt>a close(fd) on open failure may lead to unexpected behavior since the file is not even opened.</tt><br><br><tt>Also, the pattern seems to be multiple error paths with same set of instructions. Does it make sense to put them in a macro -or- better yet have fd auto cleaned using the cleanup attribute ?</tt><br><br><a href="https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html"><tt>https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html</tt></a><br><br>----------------------------------------------<br><br><tt>"openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org> wrote on 11/04/2016 06:50:36 am:<br><br>> From: OpenBMC Patches <openbmc-patches@stwcx.xyz></tt><br><tt>> To: openbmc@lists.ozlabs.org</tt><br><tt>> Cc: Ken <ken1029@gmail.com></tt><br><tt>> Date: 11/04/2016 06:51 am</tt><br><tt>> Subject: [PATCH skeleton v5 07/19] fix info memory leak</tt><br><tt>> Sent by: "openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org></tt><br><tt>> <br>> From: Ken <ken1029@gmail.com><br>> <br>> ---<br>> objects/info.c | 5 ++++-<br>> 1 file changed, 4 insertions(+), 1 deletion(-)<br>> <br>> diff --git a/objects/info.c b/objects/info.c<br>> index ab2d98c..d872c3f 100644<br>> --- a/objects/info.c<br>> +++ b/objects/info.c<br>> @@ -42,6 +42,7 @@ static int i2c_open() {<br>> fd = open(fn, O_RDWR);<br>> if (fd == -1) {<br>> LOG_ERR(errno, "Failed to open i2c device %s", fn);<br>> + close(fd);</tt><br><br><tt>I remember giving this same comment back the.. There is nothing to close if open itself fails.</tt><br><tt><br>> return -1;<br>> }<br>> <br>> @@ -108,6 +109,7 @@ static int i2c_io(int fd) {<br>> rc = ioctl(fd, I2C_RDWR, &data);<br>> if (rc < 0) {<br>> LOG_ERR(errno, "Failed to do raw io");<br>> + close(fd);<br>> return -1;<br>> }<br>> <br>> @@ -126,6 +128,7 @@ int get_hdd_status(void)<br>> }<br>> <br>> if (i2c_io(fd) < 0) {<br>> + close(fd);<br>> return -1;<br>> }<br>> <br>> @@ -145,7 +148,7 @@ int get_hdd_status(void)<br>> <br>> g_read_tmp[2]=g_read_bytes[2];<br>> g_read_tmp[3]=g_read_bytes[3];<br>> -<br>> + close(fd);<br>> }<br>> <br>> int send_esel_to_dbus(const char *desc, const char *sev, const char<br>> *details, uint8_t *debug, size_t debuglen) {<br>> -- <br>> 2.7.1<br>> <br>> <br>> _______________________________________________<br>> openbmc mailing list<br>> openbmc@lists.ozlabs.org<br>> <a href="https://lists.ozlabs.org/listinfo/openbmc">https://lists.ozlabs.org/listinfo/openbmc</a><br></tt><BR>
</body></html>