On Fri, 24 May 2019, Stephen Smalley wrote:
> I wouldn't put the variable name/field name into the error message. "could not
> allocate memory for ignoredirs" would suffice.
Agreed. I will fix it.
> > + }
> > free($3);
> > }
> > @@ -240,8 +243,8 @@ handle_unknown: HANDLE_UNKNOWN '=' ARG {
> > bzip_blocksize: BZIP_BLOCKSIZE '=' ARG {
> > int blocksize = atoi($3);
> > free($3);
> > - if (blocksize > 9)
> > - yyerror("bzip-blocksize can only be in the range 0-9");
> > + if (blocksize < 1 || blocksize > 9)
> > + yyerror("bzip-blocksize can only be in the range 1-9");
> > else
> > current_conf->bzip_blocksize = blocksize;
> > }
> > @@ -339,9 +342,18 @@ static int semanage_conf_init(semanage_conf_t * conf)
> > {
> > conf->store_type = SEMANAGE_CON_DIRECT;
> > conf->store_path = strdup(basename(selinux_policy_root()));
> > + if (conf->store_path == NULL) {
> > + return -1;
> > + }
> > conf->ignoredirs = NULL;
> > conf->store_root_path = strdup("/var/lib/selinux");
> > + if (conf->store_root_path == NULL) {
> > + return -1;
> > + }
> > conf->compiler_directory_path = strdup("/usr/libexec/selinux/hll");
> > + if (conf->compiler_directory_path == NULL) {
> > + return -1;
> > + }
> > conf->policyvers = sepol_policy_kern_vers_max();
> > conf->target_platform = SEPOL_TARGET_SELINUX;
> > conf->expand_check = 1;
> > @@ -515,7 +527,10 @@ static int parse_module_store(char *arg)
> > }
> > }
> > }
> > - return 0;
> > +
> > + if (current_conf->store_path)
> > + return 0;
> > + return -3;
> > }
> >
> > static int parse_store_root_path(char *arg)
> > @@ -526,6 +541,10 @@ static int parse_store_root_path(char *arg)
> >
> > free(current_conf->store_root_path);
> > current_conf->store_root_path = strdup(arg);
> > + if (current_conf->store_root_path == NULL) {
> > + return -2;
>
> Not sure why we are using multiple error return codes here or what they mean.
I think some of the existing code in this file used "return -1" and
"return -2", so I followed that choice just in case someone would be
interested in inspecting why the functions failed.
As of now, the callers do not care more specifically than checking for
non-zero, but if someone would run the code in e.g. a debugger, it might
be good to have separate return values for different errors - I am not
sure, though.
If you want all errors to be -1, I can of course do that?
Regards,
Jokke Hämäläinen
