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