CmdLnParser: Improve error handling - avoids dereferencing NULL when parser is misconfigured (5b95c6db) · Commits · IVAS Codec Public Collaboration / IVAS Codec

lib_util/cmdln_parser.c

+80 −36

Original line number	Diff line number	Diff line
		@@ -51,7 +51,14 @@ typedef struct
		bool hasBeenParsed;
		} Option;

		static int16_t validateNoDuplicateIds(
		/* Error enum for internal use */
		typedef enum {
		CMDLN_PARSER_ERR_OK = 0,
		CMDLN_PARSER_ERR_FAILED_PARSING = -1,
		CMDLN_PARSER_ERR_MISCONFIGURED = -2,
		} CmdLnParserError;

		static CmdLnParserError validateNoDuplicateIds(
		const OptionProps *props,
		int32_t numOpts )
		{
		@@ -62,57 +69,70 @@ static int16_t validateNoDuplicateIds(
		if ( props[i].id == props[j].id )
		{
		fprintf( stderr, "[dev] Duplicate ID == %d between options %s and %s\n", props[i].id, props[i].match, props[j].match );
		return -1;
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}
		}
		}

		return 0;
		return CMDLN_PARSER_ERR_OK;
		}

		static int16_t validateOptionProps(
		static CmdLnParserError validateOptionProps(
		OptionProps props )
		{
		/* Check required properties */
		if ( props.match == NULL )
		{
		/* TODO(sgi): Don't print out usage after this - props.match is used there */
		fprintf( stderr, "[dev] Option with ID %d - missing required property \"match\"\n", props.id );
		return -1;
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}

		if ( props.id == 0 )
		{
		fprintf( stderr, "[dev] Invalid ID for option %s. ID == %d is reserved.\n", props.match, props.id );
		return -1;
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}

		/* Check match string length */
		if ( strnlen( props.match, MAX_OPTION_MATCH_LENGTH + 1 ) > MAX_OPTION_MATCH_LENGTH )
		{
		fprintf( stderr, "[dev] Option with ID %d - match string exceeds limit of %d characters.\n", props.id, MAX_OPTION_MATCH_LENGTH );
		return -1;
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}
		if ( props.matchShort != NULL && strnlen( props.matchShort, MAX_OPTION_MATCH_LENGTH + 1 ) > MAX_OPTION_MATCH_LENGTH )
		{
		fprintf( stderr, "[dev] Option with ID %d - matchShort string exceeds limit of %d characters.\n", props.id, MAX_OPTION_MATCH_LENGTH );
		return -1;
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}

		return 0;
		return CMDLN_PARSER_ERR_OK;
		}

		/* Validate given OptionProps and use them to initialize array of Options */
		static int16_t initOpts(
		static CmdLnParserError initOpts(
		const OptionProps *options,
		const int32_t numOpts,
		Option *opts )
		{
		CmdLnParserError error;

		if ( numOpts > MAX_SUPPORTED_OPTS )
		{
		fprintf( stderr, "[dev] Number of defined options (%d) exceeds limit (%d).\n", numOpts, MAX_SUPPORTED_OPTS );
		return CMDLN_PARSER_ERR_MISCONFIGURED;
		}

		/* Check for duplicate IDs */
		if ( ( error = validateNoDuplicateIds( options, numOpts ) ) != 0 )
		{
		return error;
		}

		for ( int32_t i = 0; i < numOpts; ++i )
		{
		if ( validateOptionProps( options[i] ) != 0 )
		if ( ( error = validateOptionProps( options[i] ) ) != CMDLN_PARSER_ERR_OK )
		{
		return -1;
		return error;
		}

		Option tmp = {
		@@ -123,12 +143,6 @@ static int16_t initOpts(
		opts[i] = tmp;
		}

		/* Check for duplicate IDs */
		if ( validateNoDuplicateIds( options, numOpts ) != 0 )
		{
		return -1;
		}

		return 0;
		}

		@@ -184,7 +198,7 @@ static bool optionMatchesString(
		return false;
		}

		static int16_t parseOpts(
		static CmdLnParserError parseOpts(
		int32_t argc,
		char **argv,
		Option *opts,
		@@ -220,7 +234,7 @@ static int16_t parseOpts(
		}
		fprintf( stderr, "\n" );

		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}

		break;
		@@ -234,7 +248,7 @@ static int16_t parseOpts(
		if ( stringLooksLikeOption( argv[argIdx] ) )
		{
		fprintf( stderr, "Unknown option `%s`\n", argv[argIdx] );
		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}

		/* Otherwise, value following current option.
		@@ -246,7 +260,7 @@ static int16_t parseOpts(
		else
		{
		fprintf( stderr, "Unexpected token `%s`\n", argv[argIdx] );
		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}
		}

		@@ -257,7 +271,7 @@ static int16_t parseOpts(
		{
		if ( parseOption( currOpt->props.id, &argv[currOptIdx + 1], numValues, pOutputStruct ) != 0 )
		{
		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}
		currOpt->hasBeenParsed = true;
		}
		@@ -275,7 +289,7 @@ static int16_t parseOpts(
		{
		if ( parseOption( currOpt->props.id, &argv[currOptIdx + 1], numValues, pOutputStruct ) != 0 )
		{
		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}
		currOpt->hasBeenParsed = true;
		}
		@@ -300,10 +314,10 @@ static int16_t parseOpts(
		}
		if ( missingMandatory )
		{
		return -1;
		return CMDLN_PARSER_ERR_FAILED_PARSING;
		}

		return 0;
		return CMDLN_PARSER_ERR_OK;
		}

		static const char *getBasename(
		@@ -471,34 +485,64 @@ int16_t CmdLnParser_parseArgs(
		void *pOutputStruct,
		CmdLnParser_FnPtr_ParseOption parseOption )
		{
		assert( numOptions <= MAX_SUPPORTED_OPTS );
		CmdLnParserError error;

		/* Prepare option array */
		Option opts[MAX_SUPPORTED_OPTS];
		if ( initOpts( optionProps, numOptions, opts ) != 0 )
		if ( ( error = initOpts( optionProps, numOptions, opts ) ) != CMDLN_PARSER_ERR_OK )
		{
		goto fail;
		}

		/* Iterate over argv and parse */
		if ( parseOpts( argc, argv, opts, numOptions, pOutputStruct, parseOption ) != 0 )
		if ( ( error = parseOpts( argc, argv, opts, numOptions, pOutputStruct, parseOption ) ) != CMDLN_PARSER_ERR_OK )
		{
		goto fail;
		}

		return 0;

		fail:
		switch (error)
		{
		case CMDLN_PARSER_ERR_OK:
		break;
		case CMDLN_PARSER_ERR_MISCONFIGURED:
		fprintf( stderr, "CmdLnParser is misconfigured.\n" );
		break;
		case CMDLN_PARSER_ERR_FAILED_PARSING:
		printUsage( argv[0], optionProps, numOptions );
		return -1;
		break;
		}

		void CmdLnParser_printUsage(
		return error;
		}

		int16_t CmdLnParser_printUsage(
		char *executableName,
		const CmdLnParser_Option *options,
		const int32_t numOptions )
		{
		CmdLnParserError error;

		/* Re-use initOpts for validation only */
		Option opts[MAX_SUPPORTED_OPTS];
		if ( ( error = initOpts( options, numOptions, opts ) ) != CMDLN_PARSER_ERR_OK )
		{
		goto fail;
		}

		printUsage( executableName, options, numOptions );

		return;
		fail:
		switch (error)
		{
		case CMDLN_PARSER_ERR_OK:
		break;
		case CMDLN_PARSER_ERR_MISCONFIGURED:
		fprintf( stderr, "CmdLnParser is misconfigured.\n" );
		break;
		case CMDLN_PARSER_ERR_FAILED_PARSING:
		break;
		}

		return error;
		}

lib_util/cmdln_parser.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -53,6 +53,6 @@ typedef int16_t ( CmdLnParser_FnPtr_ParseOption )( int32_t optionId, char *opt

		int16_t CmdLnParser_parseArgs( int32_t argc, char *argv, const CmdLnParser_Option options, const int32_t numOptions, void *pOutputStruct, CmdLnParser_FnPtr_ParseOption parseOption );

		void CmdLnParser_printUsage( char executableName, const CmdLnParser_Option options, const int32_t numOptions );
		int16_t CmdLnParser_printUsage( char executableName, const CmdLnParser_Option options, const int32_t numOptions );

		#endif /* CMDLN_PARSER_H */