devname is put on heap for all cases to avoid memory
leak, and ease of use in future as well
Signed-off-by: Imran Zaman <imran.zaman at intel.com>
---
extlinux/main.c | 44 ++++++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/extlinux/main.c b/extlinux/main.c
index 55a1495..b7cdf92 100644
--- a/extlinux/main.c
+++ b/extlinux/main.c
@@ -1044,12 +1044,12 @@ err:
}
#ifndef __KLIBC__
-static const char *find_device(const char *mtab_file, dev_t dev)
+static char *find_device(const char *mtab_file, dev_t dev)
{
struct mntent *mnt;
struct stat dst;
FILE *mtab;
- const char *devname = NULL;
+ char *devname = NULL;
bool done;
mtab = setmntent(mtab_file, "r");
@@ -1131,7 +1131,7 @@ static const char *find_device(const char *mtab_file,
dev_t dev)
* On newer Linux kernels we can use sysfs to get a backwards mapping
* from device names to standard filenames
*/
-static const char *find_device_sysfs(dev_t dev)
+static char *find_device_sysfs(dev_t dev)
{
char sysname[64];
char linkname[PATH_MAX];
@@ -1281,9 +1281,20 @@ err:
return rv;
}
-static const char *get_devname(const char *path)
+static char *dupname(const char *name)
{
- const char *devname = NULL;
+ char *out = NULL;
+ int len = 0;
+ if (name)
+ len = strlen(name);
+ if (len > 0)
+ out = strndup(name, len);
+ return out;
+}
+
+static char *get_devname(const char *path)
+{
+ char *devname = NULL;
struct stat st;
struct statfs sfs;
@@ -1297,17 +1308,17 @@ static const char *get_devname(const char *path)
}
if (opt.device)
- devname = opt.device;
+ devname = strdup(opt.device);
if (!devname){
if (fs_type == BTRFS) {
/* For btrfs try to get the device name from btrfs itself */
- devname = find_device_btrfs(path);
+ devname = dupname(find_device_btrfs(path));
}
}
if (!devname) {
- devname = find_device_mountinfo(path, st.st_dev);
+ devname = dupname(find_device_mountinfo(path, st.st_dev));
}
#ifdef __KLIBC__
@@ -1326,7 +1337,7 @@ static const char *get_devname(const char *path)
}
atexit(device_cleanup); /* unlink the device node on exit */
- devname = devname_buf;
+ devname = dupname(devname_buf);
}
#else
@@ -1350,10 +1361,10 @@ static const char *get_devname(const char *path)
return devname;
}
-static int open_device(const char *path, struct stat *st, const char
**_devname)
+static int open_device(const char *path, struct stat *st, char **_devname)
{
int devfd;
- const char *devname = NULL;
+ char *devname = NULL;
struct statfs sfs;
if (st)
@@ -1393,11 +1404,10 @@ static int open_device(const char *path, struct stat
*st, const char **_devname)
devfd = -1;
devname = get_devname(path);
- if (_devname)
- *_devname = devname;
if ((devfd = open(devname, O_RDWR | O_SYNC)) < 0) {
fprintf(stderr, "%s: cannot open device %s\n", program, devname);
+ free(devname);
return -1;
}
@@ -1405,9 +1415,14 @@ static int open_device(const char *path, struct stat *st,
const char **_devname)
if (validate_device(path, devfd)) {
fprintf(stderr, "%s: path %s doesn't match device %s\n",
program, path, devname);
+ free(devname);
close(devfd);
return -1;
}
+ if (_devname)
+ *_devname = devname;
+ else
+ free(devname);
return devfd;
}
@@ -1468,7 +1483,7 @@ static int install_loader(const char *path, int
update_only)
{
struct stat st, fst;
int devfd, rv;
- const char *devname;
+ char *devname = NULL;
devfd = open_device(path, &st, &devname);
if (devfd < 0)
@@ -1508,6 +1523,7 @@ static int install_loader(const char *path, int
update_only)
sync();
rv = install_bootblock(devfd, devname);
+ free(devname);
close(devfd);
sync();
--
1.9.1
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Thu, 10 Sep 2015 10:55:04 +0300 Imran Zaman via Syslinux <syslinux at zytor.com> wrote:> devname is put on heap for all cases to avoid memory > leak, and ease of use in future as well > > Signed-off-by: Imran Zaman <imran.zaman at intel.com>Reviewed-by: Paulo Alcantara <pcacjr at zytor.com> Thanks, Paulo> --- > extlinux/main.c | 44 ++++++++++++++++++++++++++++++-------------- > 1 file changed, 30 insertions(+), 14 deletions(-) > > diff --git a/extlinux/main.c b/extlinux/main.c > index 55a1495..b7cdf92 100644 > --- a/extlinux/main.c > +++ b/extlinux/main.c > @@ -1044,12 +1044,12 @@ err: > } > > #ifndef __KLIBC__ > -static const char *find_device(const char *mtab_file, dev_t dev) > +static char *find_device(const char *mtab_file, dev_t dev) > { > struct mntent *mnt; > struct stat dst; > FILE *mtab; > - const char *devname = NULL; > + char *devname = NULL; > bool done; > > mtab = setmntent(mtab_file, "r"); > @@ -1131,7 +1131,7 @@ static const char *find_device(const char > *mtab_file, dev_t dev) > * On newer Linux kernels we can use sysfs to get a backwards mapping > * from device names to standard filenames > */ > -static const char *find_device_sysfs(dev_t dev) > +static char *find_device_sysfs(dev_t dev) > { > char sysname[64]; > char linkname[PATH_MAX]; > @@ -1281,9 +1281,20 @@ err: > return rv; > } > > -static const char *get_devname(const char *path) > +static char *dupname(const char *name) > { > - const char *devname = NULL; > + char *out = NULL; > + int len = 0; > + if (name) > + len = strlen(name); > + if (len > 0) > + out = strndup(name, len); > + return out; > +} > + > +static char *get_devname(const char *path) > +{ > + char *devname = NULL; > struct stat st; > struct statfs sfs; > > @@ -1297,17 +1308,17 @@ static const char *get_devname(const char > *path) } > > if (opt.device) > - devname = opt.device; > + devname = strdup(opt.device); > > if (!devname){ > if (fs_type == BTRFS) { > /* For btrfs try to get the device name from btrfs > itself */ > - devname = find_device_btrfs(path); > + devname = dupname(find_device_btrfs(path)); > } > } > > if (!devname) { > - devname = find_device_mountinfo(path, st.st_dev); > + devname = dupname(find_device_mountinfo(path, st.st_dev)); > } > > #ifdef __KLIBC__ > @@ -1326,7 +1337,7 @@ static const char *get_devname(const char *path) > } > > atexit(device_cleanup); /* unlink the device node on > exit */ > - devname = devname_buf; > + devname = dupname(devname_buf); > } > > #else > @@ -1350,10 +1361,10 @@ static const char *get_devname(const char > *path) return devname; > } > > -static int open_device(const char *path, struct stat *st, const char > **_devname) +static int open_device(const char *path, struct stat > *st, char **_devname) { > int devfd; > - const char *devname = NULL; > + char *devname = NULL; > struct statfs sfs; > > if (st) > @@ -1393,11 +1404,10 @@ static int open_device(const char *path, > struct stat *st, const char **_devname) > devfd = -1; > devname = get_devname(path); > - if (_devname) > - *_devname = devname; > > if ((devfd = open(devname, O_RDWR | O_SYNC)) < 0) { > fprintf(stderr, "%s: cannot open device %s\n", program, > devname); > + free(devname); > return -1; > } > > @@ -1405,9 +1415,14 @@ static int open_device(const char *path, > struct stat *st, const char **_devname) if (validate_device(path, > devfd)) { fprintf(stderr, "%s: path %s doesn't match device %s\n", > program, path, devname); > + free(devname); > close(devfd); > return -1; > } > + if (_devname) > + *_devname = devname; > + else > + free(devname); > return devfd; > } > > @@ -1468,7 +1483,7 @@ static int install_loader(const char *path, int > update_only) { > struct stat st, fst; > int devfd, rv; > - const char *devname; > + char *devname = NULL; > > devfd = open_device(path, &st, &devname); > if (devfd < 0) > @@ -1508,6 +1523,7 @@ static int install_loader(const char *path, int > update_only) > sync(); > rv = install_bootblock(devfd, devname); > + free(devname); > close(devfd); > sync(); >-- Paulo Alcantara, C.E.S.A.R Speaking for myself only.
On Fri, Sep 11, 2015 at 10:12 AM, Paulo Alcantara via Syslinux <syslinux at zytor.com> wrote:> On Thu, 10 Sep 2015 10:55:04 +0300 > Imran Zaman via Syslinux <syslinux at zytor.com> wrote: > >> devname is put on heap for all cases to avoid memory >> leak, and ease of use in future as well >> >> Signed-off-by: Imran Zaman <imran.zaman at intel.com> > > Reviewed-by: Paulo Alcantara <pcacjr at zytor.com>Regarding both versions, I'm trying to figure out if it's better to keep the const nature of this then cast to a (char *) in order to free at the right spot. -- -Gene