| Message ID | 20190520082113.29184-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | acpi: s3: fix two memory leaks on fwts_get() calls and minor clean-ups | expand |
On 2019-05-20 1:21 a.m., Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Currently the two calls to fwts_get leak the data they fetch. Fix this > by only calling fwts_get once and free'ing the string. Also split > overly wide commend line. > > Addresses-Coverity: ("Resource leak") > Fixes: 6b94bb6a4b13 ("acpi: s3: check sleep type for output messages") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/acpi/s3/s3.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c > index ff25960f..87e9fd97 100644 > --- a/src/acpi/s3/s3.c > +++ b/src/acpi/s3/s3.c > @@ -52,7 +52,12 @@ static char *s3_hook = NULL; /* Hook to run after each S3 */ > > static int s3_init(fwts_framework *fw) > { > - /* Pre-init - make sure wakealarm works so that we can wake up after suspend */ > + char *str; > + > + /* > + * Pre-init - make sure wakealarm works so that we can > + * wake up after suspend > + */ > if (fwts_wakealarm_test_firing(fw, 1) != FWTS_OK) { > fwts_log_error(fw, "Cannot automatically wake machine up - aborting Sleep test."); > fwts_failed(fw, LOG_LEVEL_MEDIUM, "BadWakeAlarmSleep", > @@ -60,10 +65,13 @@ static int s3_init(fwts_framework *fw) > return FWTS_ERROR; > } > > - if (fwts_get(PM_SUPEND_PATH) != NULL && strstr(fwts_get(PM_SUPEND_PATH), "[s2idle]")) > - strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); > - else > - strncpy(sleep_type, "S3", strlen("S3") + 1); > + str = fwts_get(PM_SUPEND_PATH); > + if (str && strstr(str, "[s2idle]")) { > + strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); > + free(str); > + } else { > + strncpy(sleep_type, "S3", strlen("S3") + 1); > + } > > return FWTS_OK; > } > Acked-by: Alex Hung <alex.hung@canonical.com>
On 5/20/19 4:21 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Currently the two calls to fwts_get leak the data they fetch. Fix this > by only calling fwts_get once and free'ing the string. Also split > overly wide commend line. > > Addresses-Coverity: ("Resource leak") > Fixes: 6b94bb6a4b13 ("acpi: s3: check sleep type for output messages") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/acpi/s3/s3.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c > index ff25960f..87e9fd97 100644 > --- a/src/acpi/s3/s3.c > +++ b/src/acpi/s3/s3.c > @@ -52,7 +52,12 @@ static char *s3_hook = NULL; /* Hook to run after each S3 */ > > static int s3_init(fwts_framework *fw) > { > - /* Pre-init - make sure wakealarm works so that we can wake up after suspend */ > + char *str; > + > + /* > + * Pre-init - make sure wakealarm works so that we can > + * wake up after suspend > + */ > if (fwts_wakealarm_test_firing(fw, 1) != FWTS_OK) { > fwts_log_error(fw, "Cannot automatically wake machine up - aborting Sleep test."); > fwts_failed(fw, LOG_LEVEL_MEDIUM, "BadWakeAlarmSleep", > @@ -60,10 +65,13 @@ static int s3_init(fwts_framework *fw) > return FWTS_ERROR; > } > > - if (fwts_get(PM_SUPEND_PATH) != NULL && strstr(fwts_get(PM_SUPEND_PATH), "[s2idle]")) > - strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); > - else > - strncpy(sleep_type, "S3", strlen("S3") + 1); > + str = fwts_get(PM_SUPEND_PATH); > + if (str && strstr(str, "[s2idle]")) { > + strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); > + free(str); > + } else { > + strncpy(sleep_type, "S3", strlen("S3") + 1); > + } > > return FWTS_OK; > } Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c index ff25960f..87e9fd97 100644 --- a/src/acpi/s3/s3.c +++ b/src/acpi/s3/s3.c @@ -52,7 +52,12 @@ static char *s3_hook = NULL; /* Hook to run after each S3 */ static int s3_init(fwts_framework *fw) { - /* Pre-init - make sure wakealarm works so that we can wake up after suspend */ + char *str; + + /* + * Pre-init - make sure wakealarm works so that we can + * wake up after suspend + */ if (fwts_wakealarm_test_firing(fw, 1) != FWTS_OK) { fwts_log_error(fw, "Cannot automatically wake machine up - aborting Sleep test."); fwts_failed(fw, LOG_LEVEL_MEDIUM, "BadWakeAlarmSleep", @@ -60,10 +65,13 @@ static int s3_init(fwts_framework *fw) return FWTS_ERROR; } - if (fwts_get(PM_SUPEND_PATH) != NULL && strstr(fwts_get(PM_SUPEND_PATH), "[s2idle]")) - strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); - else - strncpy(sleep_type, "S3", strlen("S3") + 1); + str = fwts_get(PM_SUPEND_PATH); + if (str && strstr(str, "[s2idle]")) { + strncpy(sleep_type, "s2idle", strlen("s2idle") + 1); + free(str); + } else { + strncpy(sleep_type, "S3", strlen("S3") + 1); + } return FWTS_OK; }