| Message ID | 20241108044700.37633-1-zhangjiao2@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | tools/thermal: Fix common realloc mistake | expand |
On Fri, Nov 8, 2024 at 7:39 AM zhangjiao2 <zhangjiao2@cmss.chinamobile.com> wrote: > > From: zhang jiao <zhangjiao2@cmss.chinamobile.com> > > Do not set thermometer->tz NULL when realloc failed. Presumably, this fixes a problem. What problem does it fix? > Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com> > --- > tools/thermal/thermometer/thermometer.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/thermal/thermometer/thermometer.c b/tools/thermal/thermometer/thermometer.c > index 1a87a0a77f9f..e08291a97fd8 100644 > --- a/tools/thermal/thermometer/thermometer.c > +++ b/tools/thermal/thermometer/thermometer.c > @@ -259,6 +259,7 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, > { > int fd; > char tz_path[PATH_MAX]; > + void *tmp; > > sprintf(tz_path, CLASS_THERMAL"/%s/temp", path); > > @@ -268,12 +269,13 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, > return -1; > } > > - thermometer->tz = realloc(thermometer->tz, > + tmp = realloc(thermometer->tz, > sizeof(*thermometer->tz) * (thermometer->nr_tz + 1)); > - if (!thermometer->tz) { > + if (!tmp) { > ERROR("Failed to allocate thermometer->tz\n"); > return -1; > } > + thermometer->tz = tmp; > > thermometer->tz[thermometer->nr_tz].fd_temp = fd; > thermometer->tz[thermometer->nr_tz].name = strdup(name); > -- > 2.33.0 > > > >
From: zhang jiao <zhangjiao2@cmss.chinamobile.com> On Fri, 8 Nov 2024 13:09 PM Rafael J. Wysocki <Rafael J. Wysocki> wrote: >>On Fri, Nov 8, 2024 at 7:39 AM zhangjiao2 >><zhangjiao2@cmss.chinamobile.com> wrote: >> >> From: zhang jiao <zhangjiao2@cmss.chinamobile.com> >> >> Do not set thermometer->tz NULL when realloc failed. >Presumably, this fixes a problem. >What problem does it fix? Thermometer->tz is NULL when realloc failed. This will cause memory leaks. We should use temporary variables to check the return value of realloc. >> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com> >> --- >> tools/thermal/thermometer/thermometer.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/thermal/thermometer/thermometer.c b/tools/thermal/thermometer/thermometer.c >> index 1a87a0a77f9f..e08291a97fd8 100644 >> --- a/tools/thermal/thermometer/thermometer.c >> +++ b/tools/thermal/thermometer/thermometer.c >> @@ -259,6 +259,7 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, >> { >> int fd; >> char tz_path[PATH_MAX]; >> + void *tmp; >> >> sprintf(tz_path, CLASS_THERMAL"/%s/temp", path); >> >> @@ -268,12 +269,13 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, >> return -1; >> } >> >> - thermometer->tz = realloc(thermometer->tz, >> + tmp = realloc(thermometer->tz, >> sizeof(*thermometer->tz) * (thermometer->nr_tz + 1)); >> - if (!thermometer->tz) { >> + if (!tmp) { >> ERROR("Failed to allocate thermometer->tz\n"); >> return -1; >> } >> + thermometer->tz = tmp; >> >> thermometer->tz[thermometer->nr_tz].fd_temp = fd; >> thermometer->tz[thermometer->nr_tz].name = strdup(name);
Hi Zhang, thanks for spotting the issue On 08/11/2024 05:47, zhangjiao2 wrote: > From: zhang jiao <zhangjiao2@cmss.chinamobile.com> > > Do not set thermometer->tz NULL when realloc failed. You may describe a bit more the change. eg. If the 'realloc' fails, the thermal zones pointer is set to NULL. This makes all thermal zones references which were previously successfully initialized to be lost. > Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com> > --- > tools/thermal/thermometer/thermometer.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/thermal/thermometer/thermometer.c b/tools/thermal/thermometer/thermometer.c > index 1a87a0a77f9f..e08291a97fd8 100644 > --- a/tools/thermal/thermometer/thermometer.c > +++ b/tools/thermal/thermometer/thermometer.c > @@ -259,6 +259,7 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, > { > int fd; > char tz_path[PATH_MAX]; > + void *tmp; Please use right type: struct tz *tz; > sprintf(tz_path, CLASS_THERMAL"/%s/temp", path); > > @@ -268,12 +269,13 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, > return -1; > } > > - thermometer->tz = realloc(thermometer->tz, > + tmp = realloc(thermometer->tz, > sizeof(*thermometer->tz) * (thermometer->nr_tz + 1)); > - if (!thermometer->tz) { > + if (!tmp) { > ERROR("Failed to allocate thermometer->tz\n"); > return -1; > } > + thermometer->tz = tmp; > > thermometer->tz[thermometer->nr_tz].fd_temp = fd; > thermometer->tz[thermometer->nr_tz].name = strdup(name);
diff --git a/tools/thermal/thermometer/thermometer.c b/tools/thermal/thermometer/thermometer.c index 1a87a0a77f9f..e08291a97fd8 100644 --- a/tools/thermal/thermometer/thermometer.c +++ b/tools/thermal/thermometer/thermometer.c @@ -259,6 +259,7 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, { int fd; char tz_path[PATH_MAX]; + void *tmp; sprintf(tz_path, CLASS_THERMAL"/%s/temp", path); @@ -268,12 +269,13 @@ static int thermometer_add_tz(const char *path, const char *name, int polling, return -1; } - thermometer->tz = realloc(thermometer->tz, + tmp = realloc(thermometer->tz, sizeof(*thermometer->tz) * (thermometer->nr_tz + 1)); - if (!thermometer->tz) { + if (!tmp) { ERROR("Failed to allocate thermometer->tz\n"); return -1; } + thermometer->tz = tmp; thermometer->tz[thermometer->nr_tz].fd_temp = fd; thermometer->tz[thermometer->nr_tz].name = strdup(name);