Message ID | 20220630194249.886747-5-danielhb413@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | cleanup error handling in kvmppc_read_int_cpu_dt() | expand |
On 6/30/22 21:42, Daniel Henrique Barboza wrote: > This spares us a g_free() call. Let's also not use 'val' and return the > value of kvmppc_read_int_dt() directly. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > target/ppc/kvm.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 7611e9ccf6..c218380eb7 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1932,8 +1932,8 @@ static uint64_t kvmppc_read_int_dt(const char *filename, Error **errp) > */ > static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) > { > - char buf[PATH_MAX], *tmp; > - uint64_t val; > + g_autofree char *tmp = NULL; I think you need to assign g_autofree variables where they are declared. C. > + char buf[PATH_MAX]; > > if (kvmppc_find_cpu_dt(buf, sizeof(buf))) { > error_setg(errp, "Failed to read CPU property %s", propname); > @@ -1941,10 +1941,8 @@ static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) > } > > tmp = g_strdup_printf("%s/%s", buf, propname); > - val = kvmppc_read_int_dt(tmp, errp); > - g_free(tmp); > > - return val; > + return kvmppc_read_int_dt(tmp, errp); > } > > uint64_t kvmppc_get_clockfreq(void)
On 7/2/22 03:20, Cédric Le Goater wrote: > On 6/30/22 21:42, Daniel Henrique Barboza wrote: >> This spares us a g_free() call. Let's also not use 'val' and return the >> value of kvmppc_read_int_dt() directly. >> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> target/ppc/kvm.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c >> index 7611e9ccf6..c218380eb7 100644 >> --- a/target/ppc/kvm.c >> +++ b/target/ppc/kvm.c >> @@ -1932,8 +1932,8 @@ static uint64_t kvmppc_read_int_dt(const char *filename, Error **errp) >> */ >> static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) >> { >> - char buf[PATH_MAX], *tmp; >> - uint64_t val; >> + g_autofree char *tmp = NULL; > > I think you need to assign g_autofree variables where they are declared. We need to initialize the var with something, not necessarily with the value we're going to use. Initializing with 'NULL' works. Thanks, Daniel > > C. > >> + char buf[PATH_MAX]; >> if (kvmppc_find_cpu_dt(buf, sizeof(buf))) { >> error_setg(errp, "Failed to read CPU property %s", propname); >> @@ -1941,10 +1941,8 @@ static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) >> } >> tmp = g_strdup_printf("%s/%s", buf, propname); >> - val = kvmppc_read_int_dt(tmp, errp); >> - g_free(tmp); >> - return val; >> + return kvmppc_read_int_dt(tmp, errp); >> } >> uint64_t kvmppc_get_clockfreq(void) >
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 7611e9ccf6..c218380eb7 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1932,8 +1932,8 @@ static uint64_t kvmppc_read_int_dt(const char *filename, Error **errp) */ static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) { - char buf[PATH_MAX], *tmp; - uint64_t val; + g_autofree char *tmp = NULL; + char buf[PATH_MAX]; if (kvmppc_find_cpu_dt(buf, sizeof(buf))) { error_setg(errp, "Failed to read CPU property %s", propname); @@ -1941,10 +1941,8 @@ static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp) } tmp = g_strdup_printf("%s/%s", buf, propname); - val = kvmppc_read_int_dt(tmp, errp); - g_free(tmp); - return val; + return kvmppc_read_int_dt(tmp, errp); } uint64_t kvmppc_get_clockfreq(void)
This spares us a g_free() call. Let's also not use 'val' and return the value of kvmppc_read_int_dt() directly. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/kvm.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)