| Message ID | 20230512110730.78672-4-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a7e975285c9e |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | [RFC,1/4] semodule_expand: update | expand |
On Fri, May 12, 2023 at 7:22 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > Drop unnecessary declarations. > Check closing file for incomplete write. > Rework resource cleanup, so that all files and allocated memory are > released in all branches, useful to minimize reports while debugging > libsepol under valgrind(8) or sanitizers. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > .../semodule_package/semodule_unpackage.c | 122 +++++++++++------- > 1 file changed, 75 insertions(+), 47 deletions(-) > > diff --git a/semodule-utils/semodule_package/semodule_unpackage.c b/semodule-utils/semodule_package/semodule_unpackage.c > index b8c4fbce..21c97953 100644 > --- a/semodule-utils/semodule_package/semodule_unpackage.c > +++ b/semodule-utils/semodule_package/semodule_unpackage.c > @@ -11,8 +11,7 @@ > #include <fcntl.h> > #include <errno.h> > > -char *progname = NULL; > -extern char *optarg; > +static const char *progname = NULL; Can we get rid of the global here and just pass in the program name? > > static __attribute__((__noreturn__)) void usage(void) > { > @@ -20,84 +19,113 @@ static __attribute__((__noreturn__)) void usage(void) > exit(1); Same comment as patch 1 about removing "exit(1)" > } > > -static int file_to_policy_file(const char *filename, struct sepol_policy_file **pf, const char *mode) > -{ > - FILE *f; > - > - if (sepol_policy_file_create(pf)) { > - fprintf(stderr, "%s: Out of memory\n", progname); > - return -1; > - } > - > - f = fopen(filename, mode); > - if (!f) { > - fprintf(stderr, "%s: Could not open file %s: %s\n", progname, strerror(errno), filename); > - return -1; > - } > - sepol_policy_file_set_fp(*pf, f); > - return 0; > -} > - > int main(int argc, char **argv) > { > - struct sepol_module_package *pkg; > - struct sepol_policy_file *in, *out; > - FILE *fp; > + struct sepol_module_package *pkg = NULL; > + struct sepol_policy_file *in = NULL, *out = NULL; > + FILE *fp = NULL; > size_t len; > - char *ppfile, *modfile, *fcfile = NULL, *fcdata; > + const char *ppfile, *modfile, *fcfile = NULL, *fcdata; > + int ret; > > progname = argv[0]; > > - if (argc < 3) { > + if (argc < 3) > usage(); > - exit(1); > - } > > ppfile = argv[1]; > modfile = argv[2]; > if (argc >= 4) > fcfile = argv[3]; > > - if (file_to_policy_file(ppfile, &in, "r")) > - exit(1); > - > if (sepol_module_package_create(&pkg)) { > - fprintf(stderr, "%s: Out of memory\n", progname); > - exit(1); > + fprintf(stderr, "%s: Out of memory\n", progname); > + goto failure; > + } > + > + if (sepol_policy_file_create(&in)) { > + fprintf(stderr, "%s: Out of memory\n", progname); > + goto failure; > } > > + fp = fopen(ppfile, "r"); > + if (!fp) { > + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, ppfile, strerror(errno)); > + goto failure; > + } > + sepol_policy_file_set_fp(in, fp); > + > if (sepol_module_package_read(pkg, in, 0) == -1) { > - fprintf(stderr, "%s: Error while reading policy module from %s\n", > + fprintf(stderr, "%s: Error while reading policy module from %s\n", > progname, ppfile); > - exit(1); > + goto failure; > } > > - if (file_to_policy_file(modfile, &out, "w")) > - exit(1); > + sepol_policy_file_free(in); > + in = NULL; > + fclose(fp); > + fp = NULL; > > - if (sepol_policydb_write(sepol_module_package_get_policy(pkg), out)) { > - fprintf(stderr, "%s: Error while writing module to %s\n", progname, modfile); > - exit(1); > - } > + if (sepol_policy_file_create(&out)) { > + fprintf(stderr, "%s: Out of memory\n", progname); > + goto failure; > + } > + > + fp = fopen(modfile, "w"); > + if (!fp) { > + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, modfile, strerror(errno)); > + goto failure; > + } > + sepol_policy_file_set_fp(out, fp); > + > + if (sepol_policydb_write(sepol_module_package_get_policy(pkg), out)) { > + fprintf(stderr, "%s: Error while writing module to %s\n", progname, modfile); > + goto failure; > + } > + > + ret = fclose(fp); > + fp = NULL; > + if (ret) { > + fprintf(stderr, "%s: Error while closing file %s: %s\n", progname, modfile, strerror(errno)); > + goto failure; > + } > > - sepol_policy_file_free(in); > sepol_policy_file_free(out); > + out = NULL; > > len = sepol_module_package_get_file_contexts_len(pkg); > if (fcfile && len) { > fp = fopen(fcfile, "w"); > if (!fp) { > - fprintf(stderr, "%s: Could not open file %s: %s\n", progname, strerror(errno), fcfile); > - exit(1); > + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, fcfile, strerror(errno)); > + goto failure; > } > fcdata = sepol_module_package_get_file_contexts(pkg); > if (fwrite(fcdata, 1, len, fp) != len) { > - fprintf(stderr, "%s: Could not write file %s: %s\n", progname, strerror(errno), fcfile); > - exit(1); > + fprintf(stderr, "%s: Could not write file %s: %s\n", progname, fcfile, strerror(errno)); > + goto failure; > + } > + > + ret = fclose(fp); > + fp = NULL; > + if (ret) { > + fprintf(stderr, "%s: Could not close file %s: %s\n", progname, fcfile, strerror(errno)); > + goto failure; > } > - fclose(fp); > } > > + ret = EXIT_SUCCESS; > + > +cleanup: > + if (fp) > + fclose(fp); > + sepol_policy_file_free(out); > sepol_module_package_free(pkg); > - exit(0); > + sepol_policy_file_free(in); > + > + return ret; > + > +failure: > + ret = EXIT_FAILURE; > + goto cleanup; Same comment as patch 1 about ending with "return ret". Thanks, Jim > } > -- > 2.40.1 >
diff --git a/semodule-utils/semodule_package/semodule_unpackage.c b/semodule-utils/semodule_package/semodule_unpackage.c index b8c4fbce..21c97953 100644 --- a/semodule-utils/semodule_package/semodule_unpackage.c +++ b/semodule-utils/semodule_package/semodule_unpackage.c @@ -11,8 +11,7 @@ #include <fcntl.h> #include <errno.h> -char *progname = NULL; -extern char *optarg; +static const char *progname = NULL; static __attribute__((__noreturn__)) void usage(void) { @@ -20,84 +19,113 @@ static __attribute__((__noreturn__)) void usage(void) exit(1); } -static int file_to_policy_file(const char *filename, struct sepol_policy_file **pf, const char *mode) -{ - FILE *f; - - if (sepol_policy_file_create(pf)) { - fprintf(stderr, "%s: Out of memory\n", progname); - return -1; - } - - f = fopen(filename, mode); - if (!f) { - fprintf(stderr, "%s: Could not open file %s: %s\n", progname, strerror(errno), filename); - return -1; - } - sepol_policy_file_set_fp(*pf, f); - return 0; -} - int main(int argc, char **argv) { - struct sepol_module_package *pkg; - struct sepol_policy_file *in, *out; - FILE *fp; + struct sepol_module_package *pkg = NULL; + struct sepol_policy_file *in = NULL, *out = NULL; + FILE *fp = NULL; size_t len; - char *ppfile, *modfile, *fcfile = NULL, *fcdata; + const char *ppfile, *modfile, *fcfile = NULL, *fcdata; + int ret; progname = argv[0]; - if (argc < 3) { + if (argc < 3) usage(); - exit(1); - } ppfile = argv[1]; modfile = argv[2]; if (argc >= 4) fcfile = argv[3]; - if (file_to_policy_file(ppfile, &in, "r")) - exit(1); - if (sepol_module_package_create(&pkg)) { - fprintf(stderr, "%s: Out of memory\n", progname); - exit(1); + fprintf(stderr, "%s: Out of memory\n", progname); + goto failure; + } + + if (sepol_policy_file_create(&in)) { + fprintf(stderr, "%s: Out of memory\n", progname); + goto failure; } + fp = fopen(ppfile, "r"); + if (!fp) { + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, ppfile, strerror(errno)); + goto failure; + } + sepol_policy_file_set_fp(in, fp); + if (sepol_module_package_read(pkg, in, 0) == -1) { - fprintf(stderr, "%s: Error while reading policy module from %s\n", + fprintf(stderr, "%s: Error while reading policy module from %s\n", progname, ppfile); - exit(1); + goto failure; } - if (file_to_policy_file(modfile, &out, "w")) - exit(1); + sepol_policy_file_free(in); + in = NULL; + fclose(fp); + fp = NULL; - if (sepol_policydb_write(sepol_module_package_get_policy(pkg), out)) { - fprintf(stderr, "%s: Error while writing module to %s\n", progname, modfile); - exit(1); - } + if (sepol_policy_file_create(&out)) { + fprintf(stderr, "%s: Out of memory\n", progname); + goto failure; + } + + fp = fopen(modfile, "w"); + if (!fp) { + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, modfile, strerror(errno)); + goto failure; + } + sepol_policy_file_set_fp(out, fp); + + if (sepol_policydb_write(sepol_module_package_get_policy(pkg), out)) { + fprintf(stderr, "%s: Error while writing module to %s\n", progname, modfile); + goto failure; + } + + ret = fclose(fp); + fp = NULL; + if (ret) { + fprintf(stderr, "%s: Error while closing file %s: %s\n", progname, modfile, strerror(errno)); + goto failure; + } - sepol_policy_file_free(in); sepol_policy_file_free(out); + out = NULL; len = sepol_module_package_get_file_contexts_len(pkg); if (fcfile && len) { fp = fopen(fcfile, "w"); if (!fp) { - fprintf(stderr, "%s: Could not open file %s: %s\n", progname, strerror(errno), fcfile); - exit(1); + fprintf(stderr, "%s: Could not open file %s: %s\n", progname, fcfile, strerror(errno)); + goto failure; } fcdata = sepol_module_package_get_file_contexts(pkg); if (fwrite(fcdata, 1, len, fp) != len) { - fprintf(stderr, "%s: Could not write file %s: %s\n", progname, strerror(errno), fcfile); - exit(1); + fprintf(stderr, "%s: Could not write file %s: %s\n", progname, fcfile, strerror(errno)); + goto failure; + } + + ret = fclose(fp); + fp = NULL; + if (ret) { + fprintf(stderr, "%s: Could not close file %s: %s\n", progname, fcfile, strerror(errno)); + goto failure; } - fclose(fp); } + ret = EXIT_SUCCESS; + +cleanup: + if (fp) + fclose(fp); + sepol_policy_file_free(out); sepol_module_package_free(pkg); - exit(0); + sepol_policy_file_free(in); + + return ret; + +failure: + ret = EXIT_FAILURE; + goto cleanup; }
Drop unnecessary declarations. Check closing file for incomplete write. Rework resource cleanup, so that all files and allocated memory are released in all branches, useful to minimize reports while debugging libsepol under valgrind(8) or sanitizers. Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- .../semodule_package/semodule_unpackage.c | 122 +++++++++++------- 1 file changed, 75 insertions(+), 47 deletions(-)