Message ID | 1361757620-23318-3-git-send-email-cardoe@cardoe.com |
---|---|
State | New |
Headers | show |
On 02/24/2013 09:00 PM, Doug Goldstein wrote: > Handle errors and cleanup from the error in a unified place for > parse_acl_file(). > > Signed-off-by: Doug Goldstein <cardoe@cardoe.com> > CC: Anthony Liguori <aliguori@us.ibm.com> > CC: Richa Marwaha <rmarwah@linux.vnet.ibm.com> > CC: Corey Bryant <coreyb@linux.vnet.ibm.com> > TO: qemu-devel@nongnu.org > --- > qemu-bridge-helper.c | 32 +++++++++++++++++++++----------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c > index b8771a3..d95e760 100644 > --- a/qemu-bridge-helper.c > +++ b/qemu-bridge-helper.c > @@ -96,6 +96,9 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > FILE *f; > char line[4096]; > ACLRule *acl_rule; > + struct dirent **include_list = NULL; > + int i, include_count = 0; > + char *conf_file = NULL; > > f = fopen(filename, "r"); > if (f == NULL) { > @@ -105,9 +108,6 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > while (fgets(line, sizeof(line), f) != NULL) { > char *ptr = line; > char *cmd, *arg, *argend; > - struct dirent **include_list = NULL; > - int i, include_count; > - char *conf_file; > > while (isspace(*ptr)) { > ptr++; > @@ -126,9 +126,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > > if (arg == NULL) { > fprintf(stderr, "Invalid config line:\n %s\n", line); > - fclose(f); > errno = EINVAL; > - return -1; > + goto cleanup; > } > > *arg = 0; > @@ -167,8 +166,7 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > if (include_count < 0) { > fprintf(stderr, "Unable to retrieve conf files from '%s': %s\n", > arg, strerror(errno)); > - fclose(f); > - return -1; > + goto cleanup; > } > > for (i = 0; i < include_count; i++) { > @@ -177,9 +175,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > fprintf(stderr, "Failed to allocate memory for " > "file path: %s/%s\n", > arg, include_list[i]->d_name); > - fclose(f); > errno = ENOMEM; > - return -1; > + goto cleanup; > } > > parse_acl_file(conf_file, acl_list); > @@ -197,15 +194,28 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) > parse_acl_file(arg, acl_list); > } else { > fprintf(stderr, "Unknown command `%s'\n", cmd); > - fclose(f); > errno = EINVAL; > - return -1; > + goto cleanup; > } > } > > fclose(f); > > return 0; > + > +cleanup: Maybe change this label to "failure:" since it's only a failure path? Also I think you want to save and restore errno in this path. For example if fclose() fails it will overwrite errno=EINVAL or whatever you had set it to in the above failure paths. > + > + fclose(f); > + > + if (include_list) { > + for (i = 0; i < include_count; i++) { > + if (include_list[i]) > + free(include_list[i]); > + } > + free(include_list); > + } > + > + return -1; > } > > static bool has_vnet_hdr(int fd) >
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index b8771a3..d95e760 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -96,6 +96,9 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) FILE *f; char line[4096]; ACLRule *acl_rule; + struct dirent **include_list = NULL; + int i, include_count = 0; + char *conf_file = NULL; f = fopen(filename, "r"); if (f == NULL) { @@ -105,9 +108,6 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) while (fgets(line, sizeof(line), f) != NULL) { char *ptr = line; char *cmd, *arg, *argend; - struct dirent **include_list = NULL; - int i, include_count; - char *conf_file; while (isspace(*ptr)) { ptr++; @@ -126,9 +126,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) if (arg == NULL) { fprintf(stderr, "Invalid config line:\n %s\n", line); - fclose(f); errno = EINVAL; - return -1; + goto cleanup; } *arg = 0; @@ -167,8 +166,7 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) if (include_count < 0) { fprintf(stderr, "Unable to retrieve conf files from '%s': %s\n", arg, strerror(errno)); - fclose(f); - return -1; + goto cleanup; } for (i = 0; i < include_count; i++) { @@ -177,9 +175,8 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) fprintf(stderr, "Failed to allocate memory for " "file path: %s/%s\n", arg, include_list[i]->d_name); - fclose(f); errno = ENOMEM; - return -1; + goto cleanup; } parse_acl_file(conf_file, acl_list); @@ -197,15 +194,28 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) parse_acl_file(arg, acl_list); } else { fprintf(stderr, "Unknown command `%s'\n", cmd); - fclose(f); errno = EINVAL; - return -1; + goto cleanup; } } fclose(f); return 0; + +cleanup: + + fclose(f); + + if (include_list) { + for (i = 0; i < include_count; i++) { + if (include_list[i]) + free(include_list[i]); + } + free(include_list); + } + + return -1; } static bool has_vnet_hdr(int fd)
Handle errors and cleanup from the error in a unified place for parse_acl_file(). Signed-off-by: Doug Goldstein <cardoe@cardoe.com> CC: Anthony Liguori <aliguori@us.ibm.com> CC: Richa Marwaha <rmarwah@linux.vnet.ibm.com> CC: Corey Bryant <coreyb@linux.vnet.ibm.com> TO: qemu-devel@nongnu.org --- qemu-bridge-helper.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)