[PATCH v2] openvz_conf: Use g_autofree

Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> --- src/openvz/openvz_conf.c | 60 ++++++++++++---------------------------- 1 file changed, 17 insertions(+), 43 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index eab3f748d0..4dbaef356c 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -163,7 +163,7 @@ openvzReadNetworkConf(virDomainDef *def, { int ret; virDomainNetDef *net = NULL; - char *temp = NULL; + g_autofree char *temp = NULL; char *token, *saveptr = NULL; /*parse routing network configuration* @@ -258,12 +258,9 @@ openvzReadNetworkConf(virDomainDef *def, } } - VIR_FREE(temp); - return 0; error: - VIR_FREE(temp); virDomainNetDefFree(net); return -1; } @@ -276,7 +273,7 @@ openvzReadFSConf(virDomainDef *def, int ret; virDomainFSDef *fs = NULL; g_autofree char *veid_str = NULL; - char *temp = NULL; + g_autofree char *temp = NULL; const char *param; unsigned long long barrier, limit; @@ -337,11 +334,8 @@ openvzReadFSConf(virDomainDef *def, VIR_APPEND_ELEMENT(def->fss, def->nfss, fs); - VIR_FREE(temp); - return 0; error: - VIR_FREE(temp); virDomainFSDefFree(fs); return -1; } @@ -351,7 +345,7 @@ static int openvzReadMemConf(virDomainDef *def, int veid) { int ret = -1; - char *temp = NULL; + g_autofree char *temp = NULL; unsigned long long barrier, limit; const char *param; long kb_per_pages; @@ -411,7 +405,6 @@ openvzReadMemConf(virDomainDef *def, int veid) ret = 0; error: - VIR_FREE(temp); return ret; } @@ -549,7 +542,7 @@ openvzWriteConfigParam(const char * conf_file, const char *param, const char *va g_autofree char *temp_file = NULL; int temp_fd = -1; FILE *fp; - char *line = NULL; + g_autofree char *line = NULL; size_t line_size = 0; temp_file = g_strdup_printf("%s.tmp", conf_file); @@ -586,12 +579,9 @@ openvzWriteConfigParam(const char * conf_file, const char *param, const char *va if (rename(temp_file, conf_file) < 0) goto error; - VIR_FREE(line); - return 0; error: - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(temp_fd); if (temp_file) @@ -602,14 +592,13 @@ openvzWriteConfigParam(const char * conf_file, const char *param, const char *va int openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value) { - char *conf_file; + g_autofree char *conf_file = NULL; int ret; if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0) return -1; ret = openvzWriteConfigParam(conf_file, param, value); - VIR_FREE(conf_file); return ret; } @@ -622,7 +611,7 @@ openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value) int openvzReadConfigParam(const char *conf_file, const char *param, char **value) { - char *line = NULL; + g_autofree char *line = NULL; size_t line_size = 0; FILE *fp; int err = 0; @@ -652,7 +641,6 @@ openvzReadConfigParam(const char *conf_file, const char *param, char **value) /* keep going - last entry wins */ } } - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); return err ? -1 : *value ? 1 : 0; @@ -672,21 +660,20 @@ openvzReadConfigParam(const char *conf_file, const char *param, char **value) int openvzReadVPSConfigParam(int vpsid, const char *param, char **value) { - char *conf_file; + g_autofree char *conf_file = NULL; int ret; if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0) return -1; ret = openvzReadConfigParam(conf_file, param, value); - VIR_FREE(conf_file); return ret; } static int openvz_copyfile(char* from_path, char* to_path) { - char *line = NULL; + g_autofree char *line = NULL; size_t line_size = 0; FILE *fp; int copy_fd; @@ -715,12 +702,9 @@ openvz_copyfile(char* from_path, char* to_path) if (VIR_CLOSE(copy_fd) < 0) goto error; - VIR_FREE(line); - return 0; error: - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(copy_fd); return -1; @@ -734,10 +718,10 @@ openvz_copyfile(char* from_path, char* to_path) int openvzCopyDefaultConfig(int vpsid) { - char *confdir = NULL; - char *default_conf_file = NULL; - char *configfile_value = NULL; - char *conf_file = NULL; + g_autofree char *confdir = NULL; + g_autofree char *default_conf_file = NULL; + g_autofree char *configfile_value = NULL; + g_autofree char *conf_file = NULL; int ret = -1; if (openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", &configfile_value) < 0) @@ -758,10 +742,6 @@ openvzCopyDefaultConfig(int vpsid) ret = 0; cleanup: - VIR_FREE(confdir); - VIR_FREE(default_conf_file); - VIR_FREE(configfile_value); - VIR_FREE(conf_file); return ret; } @@ -771,7 +751,7 @@ openvzCopyDefaultConfig(int vpsid) static int openvzLocateConfFileDefault(int vpsid, char **conffile, const char *ext) { - char *confdir; + g_autofree char *confdir = NULL; int ret = 0; confdir = openvzLocateConfDir(); @@ -780,7 +760,6 @@ openvzLocateConfFileDefault(int vpsid, char **conffile, const char *ext) *conffile = g_strdup_printf("%s/%d.%s", confdir, vpsid, ext ? ext : "conf"); - VIR_FREE(confdir); return ret; } @@ -828,8 +807,8 @@ openvz_readline(int fd, char *ptr, int maxlen) static int openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) { - char *conf_file; - char *line = NULL; + g_autofree char *conf_file = NULL; + g_autofree char *line = NULL; size_t line_size = 0; char *saveptr = NULL; char *uuidbuf; @@ -868,9 +847,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) } retval = 0; cleanup: - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); - VIR_FREE(conf_file); return retval; } @@ -881,7 +858,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) int openvzSetDefinedUUID(int vpsid, unsigned char *uuid) { - char *conf_file; + g_autofree char *conf_file = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; FILE *fp = NULL; int ret = -1; @@ -912,7 +889,6 @@ openvzSetDefinedUUID(int vpsid, unsigned char *uuid) ret = 0; cleanup: VIR_FORCE_FCLOSE(fp); - VIR_FREE(conf_file); return ret; } @@ -941,7 +917,7 @@ static int openvzAssignUUIDs(void) { g_autoptr(DIR) dp = NULL; struct dirent *dent; - char *conf_dir; + g_autofree char *conf_dir = NULL; int vpsid; char *ext; int ret = 0; @@ -951,7 +927,6 @@ static int openvzAssignUUIDs(void) return -1; if (virDirOpenQuiet(&dp, conf_dir) < 0) { - VIR_FREE(conf_dir); return 0; } @@ -964,7 +939,6 @@ static int openvzAssignUUIDs(void) openvzSetUUID(vpsid); } - VIR_FREE(conf_dir); return ret; } -- 2.34.1

On 3/14/24 21:22, Rayhan Faizel wrote:
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> --- src/openvz/openvz_conf.c | 60 ++++++++++++---------------------------- 1 file changed, 17 insertions(+), 43 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations on your first libvirt contribution! BTW: even after this patch there are some more opportunities for automatic cleanup. They may require slight code rework though. For instance, in openvzReadNetworkConf() the 'net' variable is declared for the whole function even though it's really used inside a for() loop. We can do two things: 1) decrease scope of the variable, and 2) declare it as g_autoptr(virDomainNetDef) net = NULL; These both steps guarantee that whenever controls jumps out of the loop, then virDomainNetDefFree(net) will be called. And to avoid freeing definition that was just appended to def->nets, VIR_APPEND_ELEMENT_COPY() needs to be changed to VIR_APPEND_ELEMENT() which upon successful return set net=NULL (thus rendering virDomainNetDefFree() to NOP). Simirarly, 'fs' in openvzReadFSConf() can use the same treatement. But here you may need to declare what free function corresponds to virDomainFSDef. Take a look at src/conf/domain_conf.h for inspiration. Finally, some labels are/will be onelines containing nothing but a return statement. We tend to drop these and replace 'goto $LABEL' with those return statements for easier reading of the code. Michal
participants (2)
-
Michal Prívozník
-
Rayhan Faizel