util-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] chrt: Allow optional priority for non‑prio policies without --pid
@ 2025-07-18 19:04 Madadi Vineeth Reddy
  2025-07-21  7:01 ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Madadi Vineeth Reddy @ 2025-07-18 19:04 UTC (permalink / raw)
  To: util-linux; +Cc: Karel Zak, Benno Schulenberg, Madadi Vineeth Reddy

This extends commit e7a2d62434c2
("chrt: Make priority optional for policies that don't use it")
so that priority arguments are optional even when --pid is not specified.

Before this patch:
$ chrt --other ls -lh
chrt: invalid priority argument: 'ls'
-> only "chrt --other 0 ls -lh" would work

After this patch:
$ chrt --other ls -lh
$ chrt --other 0 ls -lh
-> both now work

If an out‑of‑range priority is given, it reports an error:
$ chrt --other 1 ls -lh
unsupported priority value for the policy: 1 (see --max for valid range)

Fixes: e7a2d62434c2 ("chrt: Make priority optional for policies that don't use it")
Signed-off-by: Madadi Vineeth Reddy <vineethr@linux.ibm.com>
---
 schedutils/chrt.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

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;
+}
+
 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");
+
 		execvp(argv[0], argv);
 		errexec(argv[0]);
 	}
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] chrt: Allow optional priority for non‑prio policies without --pid
  2025-07-18 19:04 [PATCH] chrt: Allow optional priority for non‑prio policies without --pid Madadi Vineeth Reddy
@ 2025-07-21  7:01 ` Karel Zak
  2025-07-21 15:43   ` Madadi Vineeth Reddy
  0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2025-07-21  7:01 UTC (permalink / raw)
  To: Madadi Vineeth Reddy; +Cc: util-linux, Benno Schulenberg

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.

> +
>  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").

Thanks!
    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] chrt: Allow optional priority for non‑prio policies without --pid
  2025-07-21  7:01 ` Karel Zak
@ 2025-07-21 15:43   ` Madadi Vineeth Reddy
  0 siblings, 0 replies; 3+ messages in thread
From: Madadi Vineeth Reddy @ 2025-07-21 15:43 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux, Benno Schulenberg, Madadi Vineeth Reddy

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
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-21 15:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-18 19:04 [PATCH] chrt: Allow optional priority for non‑prio policies without --pid Madadi Vineeth Reddy
2025-07-21  7:01 ` Karel Zak
2025-07-21 15:43   ` Madadi Vineeth Reddy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).