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 | 48 ++++++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/extlinux/main.c b/extlinux/main.c
index 55a1495..7bb7443 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,22 +1308,22 @@ 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__
if (!devname) {
- devname = find_device_sysfs(st.st_dev);
+ devname = dupname(find_device_sysfs(st.st_dev));
}
if (!devname) {
/* klibc doesn't have getmntent and friends; instead, just create
@@ -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
@@ -1338,7 +1349,7 @@ static const char *get_devname(const char *path)
devname = find_device("/etc/mtab", st.st_dev);
}
if (!devname) {
- devname = find_device_sysfs(st.st_dev);
+ devname = dupname(find_device_sysfs(st.st_dev));
fprintf(stderr, "%s: cannot find device for path %s\n", program,
path);
return devname;
@@ -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 Wed, 9 Sep 2015 14:00:35 +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> > --- > extlinux/main.c | 48 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 32 insertions(+), 16 deletions(-) > > diff --git a/extlinux/main.c b/extlinux/main.c > index 55a1495..7bb7443 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)Please, keep it constant. There is no real point to remove its constantness, right? If I'm not missing anything here, please do the same for the others.> { > 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,22 +1308,22 @@ 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__ > if (!devname) { > - devname = find_device_sysfs(st.st_dev); > + devname = dupname(find_device_sysfs(st.st_dev));find_device_sysfs() already returns an allocated pointer to the device name, so you're leaking memory here -- no need to call dupname().> } > if (!devname) { > /* klibc doesn't have getmntent and friends; instead, just > create @@ -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 > @@ -1338,7 +1349,7 @@ static const char *get_devname(const char *path) > devname = find_device("/etc/mtab", st.st_dev); > } > if (!devname) { > - devname = find_device_sysfs(st.st_dev); > + devname = dupname(find_device_sysfs(st.st_dev));Ditto.> > fprintf(stderr, "%s: cannot find device for path %s\n", > program, path); return devname; > @@ -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);Please, indent this snippet of code. Thanks, Paulo> 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.