From d2fe05293b267967ccd22055f48f983ff73e603b Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Thu, 11 Dec 2025 14:27:46 +0100 Subject: [PATCH 01/10] Integrate mandatory parameters into CmdlnParser, improve usage printout --- apps/isar_post_rend.c | 50 +++------------------ apps/renderer.c | 65 ++++----------------------- lib_util/cmdln_parser.c | 97 +++++++++++++++++++++++++++++------------ lib_util/cmdln_parser.h | 5 ++- 4 files changed, 85 insertions(+), 132 deletions(-) diff --git a/apps/isar_post_rend.c b/apps/isar_post_rend.c index dd99da223f..309227dc9a 100644 --- a/apps/isar_post_rend.c +++ b/apps/isar_post_rend.c @@ -57,8 +57,7 @@ * Local constants *------------------------------------------------------------------------------------------*/ -#define POST_REND_MAX_CLI_ARG_LENGTH ( FILENAME_MAX ) -#define POST_REND_NUM_MANDATORY_CMD_LINE_PARAMS 3 +#define POST_REND_MAX_CLI_ARG_LENGTH ( FILENAME_MAX ) #define ISAR_MAX16B_FLT 32767.0f #define ISAR_MIN16B_FLT ( -32768.0f ) @@ -140,18 +139,21 @@ static const CmdLnParser_Option cliOptions[] = { .match = "input_file", .matchShort = "i", .description = "Path to the input file (WAV or raw PCM with BINAURAL_SPLIT_PCM input format, coded ISAR file with BINAURAL_SPLIT_CODED)", + .isMandatory = true, }, { .id = CmdLnOptionId_inputFormat, .match = "input_format", .matchShort = "if", .description = "Audio format of input file (e.g. BINAURAL_SPLIT_CODED, BINAURAL_SPLIT_PCM, ... Use -l for a list)", + .isMandatory = true, }, { .id = CmdLnOptionId_outputFile, .match = "output_file", .matchShort = "o", .description = "Path to the output file", + .isMandatory = true, }, { .id = CmdLnOptionId_inputMetadata, @@ -394,41 +396,6 @@ static IVAS_AUDIO_CONFIG parseAudioConfig( } -static bool checkRequiredArgs( - CmdlnArgs args ) -{ - const CmdLnParser_Option *tmpOption; - - /* Check required arguments */ - bool missingRequiredArg = false; - if ( isEmptyString( args.inputFilePath ) ) - { - tmpOption = findOptionById( CmdLnOptionId_inputFile ); - fprintf( stderr, "Missing mandatory parameter: --%s/-%s\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - - if ( args.inConfig.numBinBuses == 0 ) - { - tmpOption = findOptionById( CmdLnOptionId_inputFormat ); - fprintf( stderr, "Missing mandatory parameter: --%s/-%s\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - - if ( isEmptyString( args.outputFilePath ) ) - { - tmpOption = findOptionById( CmdLnOptionId_outputFile ); - fprintf( stderr, "Missing mandatory parameter: --%s/-%s\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - - if ( missingRequiredArg ) - { - CmdLnParser_printUsage( args.executableName, cliOptions, numCliOptions, POST_REND_NUM_MANDATORY_CMD_LINE_PARAMS ); - } - - return !missingRequiredArg; -} static CmdlnArgs defaultArgs( @@ -643,7 +610,7 @@ static int16_t parseOption( strncpy( args->srParamsFilePath, optionValues[0], POST_REND_MAX_CLI_ARG_LENGTH - 1 ); break; case CmdLnOptionId_help: - CmdLnParser_printUsage( args->executableName, cliOptions, numCliOptions, POST_REND_NUM_MANDATORY_CMD_LINE_PARAMS ); + CmdLnParser_printUsage( args->executableName, cliOptions, numCliOptions ); exit( 0 ); default: fprintf( stderr, "Error: Incorrect or invalid command-line usage!\n" ); @@ -661,12 +628,7 @@ static CmdlnArgs parseCmdlnArgs( { CmdlnArgs args = defaultArgs( argv[0] ); - if ( CmdLnParser_parseArgs( argc, argv, cliOptions, numCliOptions, &args, parseOption, POST_REND_NUM_MANDATORY_CMD_LINE_PARAMS ) != 0 ) - { - exit( -1 ); /* Error printout handled by failing function */ - } - - if ( !checkRequiredArgs( args ) ) + if ( CmdLnParser_parseArgs( argc, argv, cliOptions, numCliOptions, &args, parseOption ) != 0 ) { exit( -1 ); /* Error printout handled by failing function */ } diff --git a/apps/renderer.c b/apps/renderer.c index bba0c4c231..83315bc4cf 100644 --- a/apps/renderer.c +++ b/apps/renderer.c @@ -65,10 +65,9 @@ * Local constants *------------------------------------------------------------------------------------------*/ -#define RENDERER_MAX_CLI_ARG_LENGTH ( FILENAME_MAX ) -#define RENDERER_MAX_METADATA_LENGTH 8192 -#define RENDERER_MAX_METADATA_LINE_LENGTH 1024 -#define RENDERER_NUM_MANDATORY_CMD_LINE_PARAMS 1000 +#define RENDERER_MAX_CLI_ARG_LENGTH ( FILENAME_MAX ) +#define RENDERER_MAX_METADATA_LENGTH 8192 +#define RENDERER_MAX_METADATA_LINE_LENGTH 1024 #define IVAS_MAX16B_FLT 32767.0f #define IVAS_MIN16B_FLT ( -32768.0f ) @@ -236,12 +235,14 @@ static const CmdLnParser_Option cliOptions[] = { .match = "input_file", .matchShort = "i", .description = "Path to the input file (WAV, raw PCM or scene description file)", + .isMandatory = true, }, { .id = CmdLnOptionId_inputFormat, .match = "input_format", .matchShort = "if", .description = "Audio format of input file (e.g. 5_1 or HOA3 or META,\nuse -l for a list)", + .isMandatory = true, }, { .id = CmdLnOptionId_inputMetadata, @@ -254,12 +255,14 @@ static const CmdLnParser_Option cliOptions[] = { .match = "output_file", .matchShort = "o", .description = "Path to the output file", + .isMandatory = true, }, { .id = CmdLnOptionId_outputFormat, .match = "output_format", .matchShort = "of", .description = "Output format to render.\nAlternatively, can be a custom loudspeaker layout file", + .isMandatory = true, }, { .id = CmdLnOptionId_sampleRate, @@ -2596,53 +2599,6 @@ static bool parseReverbRoomSize( return true; } - -static bool checkRequiredArgs( - CmdlnArgs args ) -{ - const CmdLnParser_Option *tmpOption; - - /* Check required arguments */ - bool missingRequiredArg = false; - if ( isEmptyString( args.inputFilePath ) ) - { - tmpOption = findOptionById( CmdLnOptionId_inputFile ); - fprintf( stderr, "Missing required argument: %s (%s)\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - - const bool singleInputSpecified = args.inConfig.numAudioObjects != 0 || - args.inConfig.numAmbisonicsBuses != 0 || - args.inConfig.numMultiChannelBuses != 0 || - args.inConfig.numMasaBuses != 0; - - if ( !args.sceneDescriptionInput && !singleInputSpecified ) - { - /* Neither scene description input nor single-type input was specified on command line */ - tmpOption = findOptionById( CmdLnOptionId_inputFormat ); - fprintf( stderr, "Missing required argument: %s (%s)\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - if ( isEmptyString( args.outputFilePath ) ) - { - tmpOption = findOptionById( CmdLnOptionId_outputFile ); - fprintf( stderr, "Missing required argument: %s (%s)\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - if ( args.outConfig.audioConfig == IVAS_AUDIO_CONFIG_INVALID ) - { - tmpOption = findOptionById( CmdLnOptionId_outputFormat ); - fprintf( stderr, "Missing required argument: %s (%s)\n", tmpOption->match, tmpOption->matchShort ); - missingRequiredArg = true; - } - if ( missingRequiredArg ) - { - CmdLnParser_printUsage( args.executableName, cliOptions, numCliOptions, 100 ); - } - - return !missingRequiredArg; -} - static CmdlnArgs defaultArgs( const char *executableName ) { @@ -2921,12 +2877,7 @@ static CmdlnArgs parseCmdlnArgs( { CmdlnArgs args = defaultArgs( argv[0] ); - if ( CmdLnParser_parseArgs( argc, argv, cliOptions, numCliOptions, &args, parseOption, RENDERER_NUM_MANDATORY_CMD_LINE_PARAMS ) != 0 ) - { - exit( -1 ); /* Error printout handled by failing function */ - } - - if ( !checkRequiredArgs( args ) ) + if ( CmdLnParser_parseArgs( argc, argv, cliOptions, numCliOptions, &args, parseOption ) != 0 ) { exit( -1 ); /* Error printout handled by failing function */ } diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index d51f1ff1eb..be7abc62ce 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -131,7 +131,7 @@ static int8_t stringLooksLikeOption( static const char *stringToOptionName( const char *str ) { - while ( ( *str == '-' ) && ( ( str[1] != '0' ) || ( str[1] != '1' ) ) ) + while ( *str == '-' ) { ++str; } @@ -202,7 +202,7 @@ static int16_t parseOpts( /* Check if already parsed */ if ( optToMatch->hasBeenParsed ) { - fprintf( stderr, "Duplicate option: %s (%s)\n", optToMatch->props.match, optToMatch->props.matchShort ); + fprintf( stderr, "Duplicate option provided: --%s/-%s\n", optToMatch->props.match, optToMatch->props.matchShort ); return -1; } @@ -216,7 +216,7 @@ static int16_t parseOpts( /* Invalid option */ if ( stringLooksLikeOption( argv[argIdx] ) ) { - fprintf( stderr, "Unknown option `%s`\n", stringToOptionName( argv[argIdx] ) ); + fprintf( stderr, "Unknown option `%s`\n", argv[argIdx] ); return -1; } @@ -263,6 +263,23 @@ static int16_t parseOpts( currOpt->hasBeenParsed = 1; } + /* Check mandatory options */ + bool missingMandatory = false; + for ( int32_t optIdx = 0; optIdx < numOpts; ++optIdx ) + { + Option opt = opts[optIdx]; + + if ( opt.props.isMandatory && !opt.hasBeenParsed ) + { + fprintf( stderr, "Missing mandatory parameter: --%s/-%s\n", opt.props.match, opt.props.matchShort ); + missingMandatory = true; + } + } + if ( missingMandatory ) + { + return -1; + } + return 0; } @@ -331,50 +348,74 @@ static void printOptDescriptionAligned( return; } +static void printOptions( + const OptionProps *optionProps, + const int32_t numOptions, + const bool mandatory, + const int32_t maxOptNameLength + ) +{ + const int32_t preDescriptionWhitespace = 4; + const int32_t leftColumnAdditionalChars = 7; + int32_t optNameLength; + + for ( int32_t i = 0; i < numOptions; ++i ) + { + OptionProps opt = optionProps[i]; + + if ( opt.isMandatory != mandatory ) + { + continue; + } + + optNameLength = totalOptionNameLength( optionProps[i] ); + + fprintf( stderr, " --%s, -%s", opt.match, opt.matchShort ); + + printWhitespace( maxOptNameLength - optNameLength + preDescriptionWhitespace - 2 ); + fprintf( stderr, ": " ); + printOptDescriptionAligned( opt.description, maxOptNameLength + leftColumnAdditionalChars + preDescriptionWhitespace ); + } +} + static void printUsage( const char *argv0, const OptionProps *optionProps, - const int32_t numOptions, - const int16_t numMandatoryCmdLineParams ) + const int32_t numOptions ) { int32_t optNameLength; + bool hasMandatoryOptions = false; fprintf( stderr, "\n" ); fprintf( stderr, "Usage: %s [options]\n", getBasename( argv0 ) ); fprintf( stderr, "\n" ); - /* Find option with longest name, used for pretty formatting */ int32_t maxOptNameLength = 0; for ( int32_t i = 0; i < numOptions; ++i ) { + /* Find option with longest name, used for pretty formatting */ optNameLength = totalOptionNameLength( optionProps[i] ); if ( maxOptNameLength < optNameLength ) { maxOptNameLength = optNameLength; } - } - - fprintf( stderr, "Mandatory parameters:\n" ); - - const int32_t preDescriptionWhitespace = 8; - const int32_t leftColumnAdditionalChars = 7; - for ( int32_t i = 0; i < numOptions; ++i ) - { - OptionProps opt = optionProps[i]; - optNameLength = totalOptionNameLength( optionProps[i] ); - if ( i == numMandatoryCmdLineParams ) + /* Check if mandatory parameters should be printed separately */ + if ( optionProps[i].isMandatory ) { - fprintf( stderr, "\nOptions:\n" ); + hasMandatoryOptions = true; } + } - /* TODO(sgi): make matchShort optional */ - fprintf( stderr, " --%s, -%s", opt.match, opt.matchShort ); - - printWhitespace( maxOptNameLength - optNameLength + preDescriptionWhitespace ); - printOptDescriptionAligned( opt.description, maxOptNameLength + preDescriptionWhitespace + leftColumnAdditionalChars ); + if ( hasMandatoryOptions ) + { + fprintf( stderr, "Mandatory parameters:\n---------------------\n" ); + printOptions( optionProps, numOptions, true, maxOptNameLength ); } + fprintf( stderr, "\nOptions:\n--------\n" ); + printOptions( optionProps, numOptions, false, maxOptNameLength ); + return; } @@ -384,8 +425,7 @@ int16_t CmdLnParser_parseArgs( const OptionProps *optionProps, const int32_t numOptions, void *pOutputStruct, - CmdLnParser_FnPtr_ParseOption parseOption, - const int16_t numMandatoryCmdLineParams ) + CmdLnParser_FnPtr_ParseOption parseOption ) { assert( numOptions <= MAX_SUPPORTED_OPTS ); @@ -405,17 +445,16 @@ int16_t CmdLnParser_parseArgs( return 0; fail: - printUsage( argv[0], optionProps, numOptions, numMandatoryCmdLineParams ); + printUsage( argv[0], optionProps, numOptions ); return -1; } void CmdLnParser_printUsage( char *executableName, const CmdLnParser_Option *options, - const int32_t numOptions, - const int16_t numMandatoryCmdLineParams ) + const int32_t numOptions ) { - printUsage( executableName, options, numOptions, numMandatoryCmdLineParams ); + printUsage( executableName, options, numOptions ); return; } diff --git a/lib_util/cmdln_parser.h b/lib_util/cmdln_parser.h index 58deb4a41e..03289f4d29 100644 --- a/lib_util/cmdln_parser.h +++ b/lib_util/cmdln_parser.h @@ -42,13 +42,14 @@ typedef struct const char *match; const char *matchShort; const char *description; + bool isMandatory; } CmdLnParser_Option; /* Function for parsing option values into an output struct, to be implemented by the user */ typedef int16_t ( *CmdLnParser_FnPtr_ParseOption )( int32_t optionId, char **optionValues, int16_t numOptionValues, void *pOutputStruct ); -int16_t CmdLnParser_parseArgs( int32_t argc, char **argv, const CmdLnParser_Option *options, const int32_t numOptions, void *pOutputStruct, CmdLnParser_FnPtr_ParseOption parseOption, const int16_t numMandatoryCmdLineParams ); +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, const int16_t numMandatoryCmdLineParams ); +void CmdLnParser_printUsage( char *executableName, const CmdLnParser_Option *options, const int32_t numOptions ); #endif /* CMDLN_PARSER_H */ -- GitLab From 0b29b18883a25b5eea448bfb36b9364c8f64bc3f Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Thu, 11 Dec 2025 16:05:56 +0100 Subject: [PATCH 02/10] CmdLnParser: add placeholders for option values in usage printout --- apps/isar_post_rend.c | 12 ++++++++- apps/renderer.c | 29 ++++++++++++++++++-- lib_util/cmdln_parser.c | 60 ++++++++++++++++++++++++++--------------- lib_util/cmdln_parser.h | 11 ++++---- 4 files changed, 83 insertions(+), 29 deletions(-) diff --git a/apps/isar_post_rend.c b/apps/isar_post_rend.c index 309227dc9a..c67b21425c 100644 --- a/apps/isar_post_rend.c +++ b/apps/isar_post_rend.c @@ -138,6 +138,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputFile, .match = "input_file", .matchShort = "i", + .placeholder = "", .description = "Path to the input file (WAV or raw PCM with BINAURAL_SPLIT_PCM input format, coded ISAR file with BINAURAL_SPLIT_CODED)", .isMandatory = true, }, @@ -145,6 +146,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputFormat, .match = "input_format", .matchShort = "if", + .placeholder = "", .description = "Audio format of input file (e.g. BINAURAL_SPLIT_CODED, BINAURAL_SPLIT_PCM, ... Use -l for a list)", .isMandatory = true, }, @@ -152,6 +154,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_outputFile, .match = "output_file", .matchShort = "o", + .placeholder = "", .description = "Path to the output file", .isMandatory = true, }, @@ -159,24 +162,28 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputMetadata, .match = "input_metadata", .matchShort = "im", + .placeholder = "", .description = "Path to the input metadata file for BINAURAL_SPLIT_PCM input mode", }, { .id = CmdLnOptionId_sampleRate, .match = "sample_rate", .matchShort = "fs", + .placeholder = "", .description = "Input sampling rate in kHz (16, 32, 48) - required only with raw PCM inputs", }, { .id = CmdLnOptionId_trajFile, .match = "trajectory_file", .matchShort = "T", + .placeholder = "", .description = "Head rotation trajectory file for simulation of head tracking", }, { .id = CmdLnOptionId_SplitRendBFIFile, .match = "post_rend_bfi_file", .matchShort = "prbfi", + .placeholder = "", .description = "Split rendering BFI (Bad Frame Indicator) file", }, { @@ -189,6 +196,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_complexityLevel, .match = "complexity_level", .matchShort = "level", + .placeholder = "", .description = "Complexity level, level = (1, 2, 3), will be defined after characterisation.", }, { @@ -207,12 +215,14 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_framing, .match = "framing", .matchShort = "fr", - .description = "Set audio rendering frame size", + .placeholder = "", + .description = "Set audio rendering frame size in ms (5, 10, 20)", }, { .id = CmdLnOptionId_srParamsFile, .match = "sr_params", .matchShort = "s", + .placeholder = "", .description = "Path to the split rendering init params file", }, { diff --git a/apps/renderer.c b/apps/renderer.c index 83315bc4cf..3169371560 100644 --- a/apps/renderer.c +++ b/apps/renderer.c @@ -234,6 +234,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputFile, .match = "input_file", .matchShort = "i", + .placeholder = "", .description = "Path to the input file (WAV, raw PCM or scene description file)", .isMandatory = true, }, @@ -241,6 +242,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputFormat, .match = "input_format", .matchShort = "if", + .placeholder = "", .description = "Audio format of input file (e.g. 5_1 or HOA3 or META,\nuse -l for a list)", .isMandatory = true, }, @@ -248,12 +250,14 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputMetadata, .match = "input_metadata", .matchShort = "im", + .placeholder = " [...]", .description = "Space-separated list of path to metadata files for ISM/MASA/OMASA/\nOSBA/BINAURAL_SPLIT_PCM inputs. \nFor OMASA, ISM files must be specified first.", }, { .id = CmdLnOptionId_outputFile, .match = "output_file", .matchShort = "o", + .placeholder = "", .description = "Path to the output file", .isMandatory = true, }, @@ -261,6 +265,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_outputFormat, .match = "output_format", .matchShort = "of", + .placeholder = "", .description = "Output format to render.\nAlternatively, can be a custom loudspeaker layout file", .isMandatory = true, }, @@ -268,66 +273,77 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_sampleRate, .match = "sample_rate", .matchShort = "fs", + .placeholder = "", .description = "Input sampling rate in kHz (16, 32, 48) - required only with raw\nPCM inputs", }, { .id = CmdLnOptionId_trajFile, .match = "trajectory_file", .matchShort = "T", + .placeholder = "", .description = "Head rotation trajectory file for simulation of head tracking\n(only for binaural outputs)", }, { .id = CmdLnOptionId_outputMetadata, .match = "output_metadata", .matchShort = "om", + .placeholder = "", .description = "coded metadata file for BINAURAL_SPLIT_PCM output mode", }, { .id = CmdLnOptionId_SplitRendBFIFile, .match = "post_rend_bfi_file", .matchShort = "prbfi", + .placeholder = "", .description = "Split rendering option: bfi file", }, { .id = CmdLnOptionId_refRotFile, .match = "reference_rotation_file", .matchShort = "rf", + .placeholder = "", .description = "Reference rotation trajectory file for simulation of head tracking\n(only for binaural outputs)", }, { .id = CmdLnOptionId_customHrtfFile, .match = "custom_hrtf", .matchShort = "hrtf", + .placeholder = "", .description = "Custom HRTF file for binaural rendering\n(only for binaural outputs)", }, { .id = CmdLnOptionId_renderConfigFile, .match = "render_config_parameters", .matchShort = "render_config", + .placeholder = "", .description = "Binaural renderer configuration parameters in file\n(only for binaural outputs)", }, { .id = CmdLnOptionId_nonDiegeticPan, .match = "non_diegetic_panning", .matchShort = "non_diegetic_pan", + .placeholder = "", .description = "Panning mono non diegetic sound to stereo -90<= pan <= 90\nleft or l or 90->left, right or r or -90->right,\ncenter or c or 0 ->middle", }, { .id = CmdLnOptionId_orientationTracking, .match = "tracking_type", .matchShort = "otr", + .placeholder = "", .description = "Head orientation tracking type: 'none', 'ref', 'avg' or `ref_vec`\nor `ref_vec_lev` (only for binaural outputs)", }, { .id = CmdlnOptionId_lfePosition, .match = "lfe_position", .matchShort = "lp", + .placeholder = ",,", .description = "Output LFE position. Comma-delimited triplet of [gain, azimuth,\nelevation] where gain is linear (like --gain, -g) and azimuth,\nelevation are in degrees. If specified, overrides the default\nbehavior which attempts to map input to output LFE channel(s)", }, { .id = CmdlnOptionId_lfeMatrix, .match = "lfe_matrix", .matchShort = "lm", + .placeholder = "", .description = "LFE panning matrix. File (CSV table) containing a matrix of\ndimensions [ num_input_lfe x num_output_channels ] with elements\nspecifying linear routing gain (like --gain, -g). If specified,\noverrides the output LFE position option and the default\nbehavior which attempts to map input to output LFE channel(s)", }, { @@ -340,6 +356,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_complexityLevel, .match = "complexity_level", .matchShort = "level", + .placeholder = "", .description = "Complexity level, level = (1, 2, 3), will be defined after\ncharacterisation.", }, { @@ -352,6 +369,7 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_inputGain, .match = "gain", .matchShort = "g", + .placeholder = "", .description = "Input gain (linear, not in dB) to be applied to input audio file", }, { @@ -364,43 +382,50 @@ static const CmdLnParser_Option cliOptions[] = { .id = CmdLnOptionId_referenceVectorFile, .match = "reference_vector_file", .matchShort = "rvf", + .placeholder = "", .description = "Reference vector trajectory file for simulation of head tracking\n(only for binaural outputs)", }, { .id = CmdLnOptionId_exteriorOrientationFile, .match = "exterior_orientation_file", .matchShort = "exof", + .placeholder = "", .description = "External orientation trajectory file for simulation of external\norientations", }, { .id = CmdLnOptionId_framing, .match = "framing", .matchShort = "fr", - .description = "Set Render audio framing.", + .placeholder = "", + .description = "Set render audio framing in ms", }, { .id = CmdLnOptionId_syncMdDelay, .match = "sync_md_delay", .matchShort = "smd", + .placeholder = "", .description = "Metadata Synchronization Delay in ms, Default is 0. Quantized by\n5ms subframes for TDRenderer (13ms -> 10ms -> 2subframes)", }, { .id = CmdLnOptionId_directivityPatternId, .match = "ism_directivity_pattern_id", .matchShort = "dpid", + .placeholder = " [...]", .description = "Directivity pattern ID(s) = [ID1, ID2, ID3, ID4]. Space-separated\nlist of up to 4 numbers (unsigned integers) can be specified for\nBINAURAL and BINAURAL_ROOM_REVERB output.\nID1, ID2, ID3, ID4 specify the directivity pattern IDs used for\nISMs 1,2,3 and 4 respectively. \nThis option needs to be accompanied by a render_config file,\notherwise a default directivity pattern is used.", }, { .id = CmdLnOptionId_acousticEnvironmentId, .match = "acoustic_environment_id", .matchShort = "aeid", + .placeholder = "", .description = "Acoustic environment ID (number > 0) alternatively, it can be\na text file where each line contains \"ID duration\" for\nBINAURAL_ROOM_REVERB output.", }, { .id = CmdLnOptionId_roomSize, .match = "room_size", .matchShort = "rsz", - .description = "Selects default reverb based on a room size (S - small | M - medium | L - large)", + .placeholder = "", + .description = "Selects default reverb based on a room size (S - small |\nM - medium | L - large)", } }; diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index be7abc62ce..c6a593b1f6 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -304,15 +304,24 @@ static const char *getBasename( return path; } -static int32_t totalOptionNameLength( +static int32_t totalNumOptChars( const OptionProps opt ) { - return (int32_t) ( strlen( opt.match ) + strlen( opt.matchShort ) ); + int32_t len = (int32_t) strlen( opt.match ) + strlen( opt.matchShort ); + + if ( opt.placeholder != NULL ) + { + len += (int32_t) strlen( opt.placeholder ); + } + + return len; } static void printWhitespace( const int32_t n ) { + assert( n >= 0 ); + for ( int32_t i = 0; i < n; ++i ) { fprintf( stderr, " " ); @@ -352,12 +361,11 @@ static void printOptions( const OptionProps *optionProps, const int32_t numOptions, const bool mandatory, - const int32_t maxOptNameLength + const int32_t maxNumOptChars ) { - const int32_t preDescriptionWhitespace = 4; - const int32_t leftColumnAdditionalChars = 7; - int32_t optNameLength; + const int32_t descriptionColumnIdx = maxNumOptChars + 11 /* Additional chars we will print in the options column */; + int32_t numOptChars; for ( int32_t i = 0; i < numOptions; ++i ) { @@ -368,13 +376,23 @@ static void printOptions( continue; } - optNameLength = totalOptionNameLength( optionProps[i] ); + numOptChars = totalNumOptChars( optionProps[i] ); - fprintf( stderr, " --%s, -%s", opt.match, opt.matchShort ); + if ( opt.placeholder != NULL ) + { + fprintf( stderr, " --%s, -%s %s", opt.match, opt.matchShort, opt.placeholder ); + numOptChars += 8; + } + else + { + fprintf( stderr, " --%s, -%s", opt.match, opt.matchShort ); + numOptChars += 7; + } - printWhitespace( maxOptNameLength - optNameLength + preDescriptionWhitespace - 2 ); + /* Done printing options column, fill with whitespace until description column */ + printWhitespace( descriptionColumnIdx - numOptChars - 2 /* account for ": " below */ ); fprintf( stderr, ": " ); - printOptDescriptionAligned( opt.description, maxOptNameLength + leftColumnAdditionalChars + preDescriptionWhitespace ); + printOptDescriptionAligned( opt.description, descriptionColumnIdx ); } } @@ -383,21 +401,20 @@ static void printUsage( const OptionProps *optionProps, const int32_t numOptions ) { - int32_t optNameLength; - bool hasMandatoryOptions = false; - fprintf( stderr, "\n" ); fprintf( stderr, "Usage: %s [options]\n", getBasename( argv0 ) ); fprintf( stderr, "\n" ); - int32_t maxOptNameLength = 0; + int32_t numOptChars; + int32_t maxNumOptChars = 0; + bool hasMandatoryOptions = false; for ( int32_t i = 0; i < numOptions; ++i ) { - /* Find option with longest name, used for pretty formatting */ - optNameLength = totalOptionNameLength( optionProps[i] ); - if ( maxOptNameLength < optNameLength ) + /* Find option with most characters when printed, used for pretty formatting */ + numOptChars = totalNumOptChars( optionProps[i] ); + if ( maxNumOptChars < numOptChars ) { - maxOptNameLength = optNameLength; + maxNumOptChars = numOptChars; } /* Check if mandatory parameters should be printed separately */ @@ -410,11 +427,12 @@ static void printUsage( if ( hasMandatoryOptions ) { fprintf( stderr, "Mandatory parameters:\n---------------------\n" ); - printOptions( optionProps, numOptions, true, maxOptNameLength ); + printOptions( optionProps, numOptions, true, maxNumOptChars ); + fprintf( stderr, "\n" ); } - fprintf( stderr, "\nOptions:\n--------\n" ); - printOptions( optionProps, numOptions, false, maxOptNameLength ); + fprintf( stderr, "Options:\n--------\n" ); + printOptions( optionProps, numOptions, false, maxNumOptChars ); return; } diff --git a/lib_util/cmdln_parser.h b/lib_util/cmdln_parser.h index 03289f4d29..36d216859d 100644 --- a/lib_util/cmdln_parser.h +++ b/lib_util/cmdln_parser.h @@ -38,11 +38,12 @@ typedef struct { - int32_t id; - const char *match; - const char *matchShort; - const char *description; - bool isMandatory; + int32_t id; /* Unique ID for the option */ + const char *match; /* String to match, e.g. "input" here will match "--input" on CLI */ + const char *matchShort; /* Short version of the string to match, e.g. "i" here will match "-i" on CLI */ + const char *placeholder; /* If not NULL, this will follow the match string in the usage printout, e.g. "" here will print "--input " on CLI */ + const char *description; /* Description of the option for the usage printout. May contain '\n' for line breaks. */ + bool isMandatory; /* Parsing will fail if an option with `isMandatory == true` is not given */ } CmdLnParser_Option; /* Function for parsing option values into an output struct, to be implemented by the user */ -- GitLab From 8009cc260a541ac4a2224c4e6ad39ea769de3708 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Thu, 11 Dec 2025 16:48:39 +0100 Subject: [PATCH 03/10] CmdLnParser: Clean up handling of optional properties in CmdLnParser_Option --- lib_util/cmdln_parser.c | 62 ++++++++++++++++++++++++++++------------- lib_util/cmdln_parser.h | 6 ++-- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index c6a593b1f6..7123f46da3 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -120,7 +120,7 @@ static int16_t initOpts( static int8_t stringLooksLikeOption( const char *str ) { - if ( ( str[0] == '-' ) && is_number( str ) == false ) + if ( ( str[0] == '-' ) && !is_number( str ) ) { return 1; } @@ -150,26 +150,32 @@ static int8_t optionMatchesString( const char *optionName = stringToOptionName( str ); - char optionName_to_upper[FILENAME_MAX]; + char optionName_to_upper[MAX_OPTION_LENGTH + 1]; strncpy( optionName_to_upper, optionName, sizeof( optionName_to_upper ) - 1 ); optionName_to_upper[sizeof( optionName_to_upper ) - 1] = '\0'; to_upper( optionName_to_upper ); - char match_to_upper[FILENAME_MAX]; + char match_to_upper[MAX_OPTION_LENGTH + 1]; strncpy( match_to_upper, opt.props.match, sizeof( match_to_upper ) - 1 ); optionName_to_upper[sizeof( match_to_upper ) - 1] = '\0'; to_upper( match_to_upper ); - - char matchShort_to_upper[FILENAME_MAX]; - strncpy( matchShort_to_upper, opt.props.matchShort, sizeof( matchShort_to_upper ) - 1 ); - optionName_to_upper[sizeof( matchShort_to_upper ) - 1] = '\0'; - to_upper( matchShort_to_upper ); - - if ( strncmp( optionName_to_upper, match_to_upper, MAX_OPTION_LENGTH ) == 0 || strncmp( optionName_to_upper, matchShort_to_upper, MAX_OPTION_LENGTH ) == 0 ) + if ( strncmp( optionName_to_upper, match_to_upper, MAX_OPTION_LENGTH ) == 0 ) { return 1; } + if ( opt.props.matchShort != NULL ) + { + strncpy( match_to_upper, opt.props.matchShort, sizeof( match_to_upper ) - 1 ); + optionName_to_upper[sizeof( match_to_upper ) - 1] = '\0'; + to_upper( match_to_upper ); + + if ( strncmp( optionName_to_upper, match_to_upper, MAX_OPTION_LENGTH ) == 0 ) + { + return 1; + } + } + return 0; } @@ -307,7 +313,12 @@ static const char *getBasename( static int32_t totalNumOptChars( const OptionProps opt ) { - int32_t len = (int32_t) strlen( opt.match ) + strlen( opt.matchShort ); + int32_t len = (int32_t) strlen( opt.match ); + + if ( opt.matchShort != NULL ) + { + len += (int32_t) strlen( opt.matchShort ); + } if ( opt.placeholder != NULL ) { @@ -378,21 +389,32 @@ static void printOptions( numOptChars = totalNumOptChars( optionProps[i] ); + fprintf( stderr, " --%s", opt.match ); + numOptChars += 4; + + if ( opt.matchShort != NULL ) + { + fprintf( stderr, ", -%s", opt.matchShort ); + numOptChars += 3; + } + if ( opt.placeholder != NULL ) { - fprintf( stderr, " --%s, -%s %s", opt.match, opt.matchShort, opt.placeholder ); - numOptChars += 8; + fprintf( stderr, " %s", opt.placeholder ); + numOptChars += 1; + } + + if ( opt.description != NULL ) + { + /* Done printing options column, fill with whitespace until description column */ + printWhitespace( descriptionColumnIdx - numOptChars - 2 /* account for ": " below */ ); + fprintf( stderr, ": " ); + printOptDescriptionAligned( opt.description, descriptionColumnIdx ); } else { - fprintf( stderr, " --%s, -%s", opt.match, opt.matchShort ); - numOptChars += 7; + fprintf( stderr, "\n" ); } - - /* Done printing options column, fill with whitespace until description column */ - printWhitespace( descriptionColumnIdx - numOptChars - 2 /* account for ": " below */ ); - fprintf( stderr, ": " ); - printOptDescriptionAligned( opt.description, descriptionColumnIdx ); } } diff --git a/lib_util/cmdln_parser.h b/lib_util/cmdln_parser.h index 36d216859d..ddb16c01d6 100644 --- a/lib_util/cmdln_parser.h +++ b/lib_util/cmdln_parser.h @@ -38,8 +38,10 @@ typedef struct { - int32_t id; /* Unique ID for the option */ - const char *match; /* String to match, e.g. "input" here will match "--input" on CLI */ + int32_t id; /* Unique ID for the option */ + const char *match; /* String to match, e.g. "input" here will match "--input" on CLI */ + + /* Struct members below are optional and can be omitted in option definition. If omitted, C will implicitly set them to 0. */ const char *matchShort; /* Short version of the string to match, e.g. "i" here will match "-i" on CLI */ const char *placeholder; /* If not NULL, this will follow the match string in the usage printout, e.g. "" here will print "--input " on CLI */ const char *description; /* Description of the option for the usage printout. May contain '\n' for line breaks. */ -- GitLab From b5575c2161ebc2d498e37deafa886af121297ac2 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Thu, 11 Dec 2025 17:00:12 +0100 Subject: [PATCH 04/10] CmdLnParser: Ensure match strings are within length limit --- lib_util/cmdln_parser.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index 7123f46da3..73d486313b 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -38,7 +38,7 @@ #include #define MAX_SUPPORTED_OPTS ( 1024 ) -#define MAX_OPTION_LENGTH ( 1024 ) +#define MAX_OPTION_LENGTH ( 32 ) typedef CmdLnParser_Option OptionProps; @@ -74,7 +74,7 @@ static int16_t validateOptionProps( 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 ); + fprintf( stderr, "[dev] Option with ID %d - missing required property \"match\"\n", props.id ); return -1; } @@ -84,6 +84,18 @@ static int16_t validateOptionProps( return -1; } + /* Check match string length */ + if ( strnlen( props.match, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) + { + fprintf( stderr, "[dev] Option with ID %d - match string exceeds limit of %d characters.\n", props.id, MAX_OPTION_LENGTH ); + return -1; + } + if ( props.matchShort != NULL && strnlen( props.matchShort, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) + { + fprintf( stderr, "[dev] Option with ID %d - matchShort string exceeds limit of %d characters.\n", props.id, MAX_OPTION_LENGTH ); + return -1; + } + return 0; } @@ -128,6 +140,7 @@ static int8_t stringLooksLikeOption( return 0; } +// TODO: too lenient. Should strip one '-' for short and two '-' for match static const char *stringToOptionName( const char *str ) { @@ -208,7 +221,13 @@ static int16_t parseOpts( /* Check if already parsed */ if ( optToMatch->hasBeenParsed ) { - fprintf( stderr, "Duplicate option provided: --%s/-%s\n", optToMatch->props.match, optToMatch->props.matchShort ); + fprintf( stderr, "Duplicate option provided: --%s", optToMatch->props.match ); + if ( optToMatch->props.matchShort != NULL ) + { + fprintf( stderr, "/-%s", optToMatch->props.matchShort ); + } + fprintf( stderr, "\n" ); + return -1; } @@ -277,7 +296,13 @@ static int16_t parseOpts( if ( opt.props.isMandatory && !opt.hasBeenParsed ) { - fprintf( stderr, "Missing mandatory parameter: --%s/-%s\n", opt.props.match, opt.props.matchShort ); + fprintf( stderr, "Missing mandatory parameter: --%s", opt.props.match ); + if ( opt.props.matchShort != NULL ) + { + fprintf( stderr, "/-%s", opt.props.matchShort ); + } + fprintf( stderr, "\n" ); + missingMandatory = true; } } -- GitLab From 003cbb537c2667cfab79688a8b3dc48d4f77e4a8 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Thu, 11 Dec 2025 18:08:17 +0100 Subject: [PATCH 05/10] CmdLnParser: Make option matching more robust --- lib_util/cmdln_parser.c | 55 +++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index 73d486313b..fe7e4e5bd5 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -37,8 +37,9 @@ #include #include -#define MAX_SUPPORTED_OPTS ( 1024 ) -#define MAX_OPTION_LENGTH ( 32 ) +#define MAX_SUPPORTED_OPTS ( 1024 ) +#define MAX_OPTION_MATCH_LENGTH ( 30 ) +#define MAX_OPTION_LENGTH ( MAX_OPTION_MATCH_LENGTH + 2 ) typedef CmdLnParser_Option OptionProps; @@ -85,14 +86,14 @@ static int16_t validateOptionProps( } /* Check match string length */ - if ( strnlen( props.match, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_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_LENGTH ); + fprintf( stderr, "[dev] Option with ID %d - match string exceeds limit of %d characters.\n", props.id, MAX_OPTION_MATCH_LENGTH ); return -1; } - if ( props.matchShort != NULL && strnlen( props.matchShort, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) + 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_LENGTH ); + fprintf( stderr, "[dev] Option with ID %d - matchShort string exceeds limit of %d characters.\n", props.id, MAX_OPTION_MATCH_LENGTH ); return -1; } @@ -140,18 +141,6 @@ static int8_t stringLooksLikeOption( return 0; } -// TODO: too lenient. Should strip one '-' for short and two '-' for match -static const char *stringToOptionName( - const char *str ) -{ - while ( *str == '-' ) - { - ++str; - } - - return str; -} - static int8_t optionMatchesString( Option opt, const char *str ) @@ -161,29 +150,30 @@ static int8_t optionMatchesString( return 0; } - const char *optionName = stringToOptionName( str ); + if ( strnlen( str, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) + { + /* String longer than longest possible option - not a match */ + return 0; + } - char optionName_to_upper[MAX_OPTION_LENGTH + 1]; - strncpy( optionName_to_upper, optionName, sizeof( optionName_to_upper ) - 1 ); - optionName_to_upper[sizeof( optionName_to_upper ) - 1] = '\0'; - to_upper( optionName_to_upper ); + char str_to_upper[MAX_OPTION_LENGTH + 1]; + snprintf( str_to_upper, sizeof( str_to_upper ), "%s", str ); + to_upper( str_to_upper ); char match_to_upper[MAX_OPTION_LENGTH + 1]; - strncpy( match_to_upper, opt.props.match, sizeof( match_to_upper ) - 1 ); - optionName_to_upper[sizeof( match_to_upper ) - 1] = '\0'; + snprintf( match_to_upper, sizeof( match_to_upper ), "--%s", opt.props.match ); to_upper( match_to_upper ); - if ( strncmp( optionName_to_upper, match_to_upper, MAX_OPTION_LENGTH ) == 0 ) + if ( strcmp( str_to_upper, match_to_upper ) == 0 ) { return 1; } if ( opt.props.matchShort != NULL ) { - strncpy( match_to_upper, opt.props.matchShort, sizeof( match_to_upper ) - 1 ); - optionName_to_upper[sizeof( match_to_upper ) - 1] = '\0'; + snprintf( match_to_upper, sizeof( match_to_upper ), "-%s", opt.props.matchShort ); to_upper( match_to_upper ); - if ( strncmp( optionName_to_upper, match_to_upper, MAX_OPTION_LENGTH ) == 0 ) + if ( strcmp( str_to_upper, match_to_upper ) == 0 ) { return 1; } @@ -342,12 +332,12 @@ static int32_t totalNumOptChars( if ( opt.matchShort != NULL ) { - len += (int32_t) strlen( opt.matchShort ); + len += (int32_t) strlen( opt.matchShort ); } if ( opt.placeholder != NULL ) { - len += (int32_t) strlen( opt.placeholder ); + len += (int32_t) strlen( opt.placeholder ); } return len; @@ -397,8 +387,7 @@ static void printOptions( const OptionProps *optionProps, const int32_t numOptions, const bool mandatory, - const int32_t maxNumOptChars - ) + const int32_t maxNumOptChars ) { const int32_t descriptionColumnIdx = maxNumOptChars + 11 /* Additional chars we will print in the options column */; int32_t numOptChars; -- GitLab From 8e4664ea85ab7a90025be7a35993ca68e7cfd365 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Fri, 12 Dec 2025 13:35:07 +0100 Subject: [PATCH 06/10] CmdLnParser: Use bool where appropriate --- lib_util/cmdln_parser.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index fe7e4e5bd5..dece415b4d 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -36,6 +36,8 @@ #include #include #include +#include +#include #define MAX_SUPPORTED_OPTS ( 1024 ) #define MAX_OPTION_MATCH_LENGTH ( 30 ) @@ -46,7 +48,7 @@ typedef CmdLnParser_Option OptionProps; typedef struct { OptionProps props; - int8_t hasBeenParsed; + bool hasBeenParsed; } Option; static int16_t validateNoDuplicateIds( @@ -114,7 +116,7 @@ static int16_t initOpts( } Option tmp = { - .hasBeenParsed = 0 + .hasBeenParsed = false }; tmp.props = options[i]; /* Cannot assign in aggregate initializer above - causes Visual Studio warning */ @@ -130,30 +132,30 @@ static int16_t initOpts( return 0; } -static int8_t stringLooksLikeOption( +static bool stringLooksLikeOption( const char *str ) { if ( ( str[0] == '-' ) && !is_number( str ) ) { - return 1; + return true; } - return 0; + return false; } -static int8_t optionMatchesString( +static bool optionMatchesString( Option opt, const char *str ) { if ( !stringLooksLikeOption( str ) ) { - return 0; + return false; } if ( strnlen( str, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) { /* String longer than longest possible option - not a match */ - return 0; + return false; } char str_to_upper[MAX_OPTION_LENGTH + 1]; @@ -165,7 +167,7 @@ static int8_t optionMatchesString( to_upper( match_to_upper ); if ( strcmp( str_to_upper, match_to_upper ) == 0 ) { - return 1; + return true; } if ( opt.props.matchShort != NULL ) @@ -175,11 +177,11 @@ static int8_t optionMatchesString( if ( strcmp( str_to_upper, match_to_upper ) == 0 ) { - return 1; + return true; } } - return 0; + return false; } static int16_t parseOpts( @@ -257,7 +259,7 @@ static int16_t parseOpts( { return -1; } - currOpt->hasBeenParsed = 1; + currOpt->hasBeenParsed = true; } currOpt = nextOpt; @@ -275,7 +277,7 @@ static int16_t parseOpts( { return -1; } - currOpt->hasBeenParsed = 1; + currOpt->hasBeenParsed = true; } /* Check mandatory options */ -- GitLab From 7c2523619f5b3f3fb3f9655a0172295205ad4d07 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Fri, 12 Dec 2025 15:56:59 +0100 Subject: [PATCH 07/10] CmdLnParser: Print whitespace using fprintf --- lib_util/cmdln_parser.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index dece415b4d..a91f1e5f0e 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -345,17 +345,6 @@ static int32_t totalNumOptChars( return len; } -static void printWhitespace( - const int32_t n ) -{ - assert( n >= 0 ); - - for ( int32_t i = 0; i < n; ++i ) - { - fprintf( stderr, " " ); - } -} - static void printOptDescriptionAligned( const char *descPtr, int32_t descriptionColumnIdx ) @@ -377,7 +366,7 @@ static void printOptDescriptionAligned( fprintf( stderr, "%c", *descPtr ); if ( *descPtr == '\n' ) { - printWhitespace( descriptionColumnIdx ); + fprintf( stderr, "%*s", descriptionColumnIdx, "" ); } ++descPtr; } @@ -423,8 +412,7 @@ static void printOptions( if ( opt.description != NULL ) { /* Done printing options column, fill with whitespace until description column */ - printWhitespace( descriptionColumnIdx - numOptChars - 2 /* account for ": " below */ ); - fprintf( stderr, ": " ); + fprintf( stderr, "%*s", descriptionColumnIdx - numOptChars, ": " ); printOptDescriptionAligned( opt.description, descriptionColumnIdx ); } else -- GitLab From 5b95c6dbcdc7f10971f9fb2e689ef8cff425c860 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Mon, 15 Dec 2025 10:39:12 +0100 Subject: [PATCH 08/10] CmdLnParser: Improve error handling - avoids dereferencing NULL when parser is misconfigured --- lib_util/cmdln_parser.c | 116 +++++++++++++++++++++++++++------------- lib_util/cmdln_parser.h | 2 +- 2 files changed, 81 insertions(+), 37 deletions(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index a91f1e5f0e..e97f2c3ace 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -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: - printUsage( argv[0], optionProps, numOptions ); - return -1; + 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 ); + break; + } + + return error; } -void CmdLnParser_printUsage( +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; } diff --git a/lib_util/cmdln_parser.h b/lib_util/cmdln_parser.h index ddb16c01d6..c6848f79b8 100644 --- a/lib_util/cmdln_parser.h +++ b/lib_util/cmdln_parser.h @@ -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 */ -- GitLab From cfc2ada7dfe2be786a7987561ae8f46e2eb65485 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Mon, 15 Dec 2025 12:04:51 +0100 Subject: [PATCH 09/10] CmdLnParser: Do not use strnlen - it is not always available --- lib_util/cmdl_tools.c | 25 +++++++++++++++++++++++++ lib_util/cmdl_tools.h | 2 ++ lib_util/cmdln_parser.c | 13 +++++++------ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib_util/cmdl_tools.c b/lib_util/cmdl_tools.c index 625db12733..1f551f8917 100644 --- a/lib_util/cmdl_tools.c +++ b/lib_util/cmdl_tools.c @@ -225,3 +225,28 @@ bool isEmptyString( { return str[0] == '\0'; } + + +/*---------------------------------------------------------------------* + * strLength() + * + * Get the length of a string up to a maximum length. + * + * Equivalent to strnlen, which is not part of the C standard and only provided by POSIX. + * + *---------------------------------------------------------------------*/ + +int32_t strLength( + const char *str, + int32_t maxlen ) +{ + int32_t len; + + len = 0; + while ( len < maxlen && str[len] != '\0' ) + { + len++; + } + + return len; +} diff --git a/lib_util/cmdl_tools.h b/lib_util/cmdl_tools.h index 327acb93d0..b331dea03d 100644 --- a/lib_util/cmdl_tools.h +++ b/lib_util/cmdl_tools.h @@ -50,4 +50,6 @@ void clearString( char *str ); bool isEmptyString( const char *str ); +int32_t strLength( const char *str, int32_t maxlen ); + #endif /* CMDL_TOOLS_H */ diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index e97f2c3ace..a1d6edcd75 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -52,7 +52,8 @@ typedef struct } Option; /* Error enum for internal use */ -typedef enum { +typedef enum +{ CMDLN_PARSER_ERR_OK = 0, CMDLN_PARSER_ERR_FAILED_PARSING = -1, CMDLN_PARSER_ERR_MISCONFIGURED = -2, @@ -94,12 +95,12 @@ static CmdLnParserError validateOptionProps( } /* Check match string length */ - if ( strnlen( props.match, MAX_OPTION_MATCH_LENGTH + 1 ) > MAX_OPTION_MATCH_LENGTH ) + if ( strLength( 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 CMDLN_PARSER_ERR_MISCONFIGURED; } - if ( props.matchShort != NULL && strnlen( props.matchShort, MAX_OPTION_MATCH_LENGTH + 1 ) > MAX_OPTION_MATCH_LENGTH ) + if ( props.matchShort != NULL && strLength( 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 CMDLN_PARSER_ERR_MISCONFIGURED; @@ -426,7 +427,7 @@ static void printOptions( if ( opt.description != NULL ) { /* Done printing options column, fill with whitespace until description column */ - fprintf( stderr, "%*s", descriptionColumnIdx - numOptChars, ": " ); + fprintf( stderr, "%*s", descriptionColumnIdx - numOptChars, ": " ); printOptDescriptionAligned( opt.description, descriptionColumnIdx ); } else @@ -501,7 +502,7 @@ int16_t CmdLnParser_parseArgs( } fail: - switch (error) + switch ( error ) { case CMDLN_PARSER_ERR_OK: break; @@ -533,7 +534,7 @@ int16_t CmdLnParser_printUsage( printUsage( executableName, options, numOptions ); fail: - switch (error) + switch ( error ) { case CMDLN_PARSER_ERR_OK: break; -- GitLab From 8cf192bcaa5768ad655e4ae918e79b600e608cf7 Mon Sep 17 00:00:00 2001 From: Kacper Sagnowski Date: Mon, 15 Dec 2025 12:41:34 +0100 Subject: [PATCH 10/10] CmdLnParser: Remove one more use of strnlen --- lib_util/cmdln_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib_util/cmdln_parser.c b/lib_util/cmdln_parser.c index a1d6edcd75..cd201a0cdc 100644 --- a/lib_util/cmdln_parser.c +++ b/lib_util/cmdln_parser.c @@ -167,7 +167,7 @@ static bool optionMatchesString( return false; } - if ( strnlen( str, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) + if ( strLength( str, MAX_OPTION_LENGTH + 1 ) > MAX_OPTION_LENGTH ) { /* String longer than longest possible option - not a match */ return false; -- GitLab