Re: [PATCH] chrt: Allow optional priority for non‑prio policies without --pid

[Madadi Vineeth Reddy <vineethr@linux.ibm.com>]

Hi Karel,

On 21/07/25 12:31, Karel Zak wrote:
> On Sat, Jul 19, 2025 at 12:34:39AM +0530, Madadi Vineeth Reddy wrote:
>> diff --git a/schedutils/chrt.c b/schedutils/chrt.c
>> index 10ba7fbf6..44b9b9e20 100644
>> --- a/schedutils/chrt.c
>> +++ b/schedutils/chrt.c
>> @@ -395,6 +395,18 @@ static void set_sched(struct chrt_ctl *ctl)
>>  	ctl->altered = 1;
>>  }
>>  
>> +static bool is_number(const char *s)
>> +{
>> +	if (!s || *s == '\0')
>> +		return false;
>> +
>> +	for (const char *p = s; *p; p++) {
>> +		if (!isdigit((unsigned char)*p))
>> +			return false;
>> +	}
>> +	return true;
>> +}
> 
> It seems you can replace this function with isdigit_string() from  
> include/strutils.h.
> 

Will replace it.

>> +
>>  int main(int argc, char **argv)
>>  {
>>  	struct chrt_ctl _ctl = { .pid = -1, .policy = SCHED_RR }, *ctl = &_ctl;
>> @@ -503,7 +515,7 @@ int main(int argc, char **argv)
>>  		}
>>  	}
>>  
>> -	if (argc - optind < (ctl->pid == 0 ? 1 : 2)) {
>> +	if (argc - optind < 1) {
>>  		warnx(_("too few arguments"));
>>  		errtryhelp(EXIT_FAILURE);
>>  	}
>> @@ -527,7 +539,10 @@ int main(int argc, char **argv)
>>  	if (ctl->verbose)
>>  		show_sched_info(ctl);
>>  
>> -	if (argc - optind > 1) {
>> +	bool have_prio = need_prio ||
>> +		(ctl->pid == -1 ? (optind < argc && is_number(argv[optind])) : (argc - optind > 1));
>> +
>> +	if (have_prio) {
>>  		errno = 0;
>>  		ctl->priority = strtos32_or_err(argv[optind], _("invalid priority argument"));
>>  	} else
>> @@ -568,9 +583,19 @@ int main(int argc, char **argv)
>>  		show_sched_info(ctl);
>>  
>>  	if (!ctl->pid) {
>> -		argv += optind + 1;
>> -		if (strcmp(argv[0], "--") == 0)
>> +		argv += optind;
>> +
>> +		if (need_prio)
>> +			argv++;
>> +		else if (argv[0] && is_number(argv[0]))
>> +			argv++;
>> +
>> +		if (argv[0] && strcmp(argv[0], "--") == 0)
>>  			argv++;
>> +
>> +		if (!argv[0])
>> +			errx(EXIT_FAILURE, "Missing command to execute");
> 
> You need to use _() for translation, ideally with string already used in  
> other tools. For example, _("no command specified").

Sure. Will fix it in v2. Thanks for taking a look.

--
Madadi Vineeth Reddy

> 
> Thanks!
>     Karel
>