[PATCH 4/4] chrt: don't rely on exact values of SCHED_* defines

Kang-Che Sung explorer09 at gmail.com
Sun Jan 14 05:48:57 UTC 2018


On Sun, Jan 14, 2018 at 5:30 AM, Povilas Kanapickas <povilas at radix.lt> wrote:
> --- a/util-linux/chrt.c
> +++ b/util-linux/chrt.c
> @@ -36,17 +36,20 @@
>  #include <sched.h>
>  #include "libbb.h"
>
> -static const struct {
> -       int policy;
> -       char name[sizeof("SCHED_OTHER")];
> -} policies[] = {
> -       {SCHED_OTHER, "SCHED_OTHER"},
> -       {SCHED_FIFO, "SCHED_FIFO"},
> -       {SCHED_RR, "SCHED_RR"},
> -       {SCHED_BATCH, "SCHED_BATCH"},
> -       {0 /* unused */, ""},
> -       {SCHED_IDLE, "SCHED_IDLE"}
> -};
> +static const char* get_policy_name(int pol)
> +{
> +       if (pol == SCHED_OTHER)
> +               return "SCHED_OTHER";
> +       if (pol == SCHED_FIFO)
> +               return "SCHED_FIFO";
> +       if (pol == SCHED_RR)
> +               return "SCHED_RR";
> +       if (pol == SCHED_BATCH)
> +               return "SCHED_BATCH";
> +       if (pol == SCHED_IDLE)
> +               return "SCHED_IDLE";
> +       return "";
> +}
>
>  static void show_min_max(int pol)
>  {

I don't like the chain of "if"s here. Have you checked the size changes on the
compiled code?
Since this SCHED policy constants are just enum values, what about using
"switch-case" statement? The problem of if-chains is that some compilers are
not smart at figuring out that the comparisons are just enums and can be
optimized. The switch-case statement will make it more clear.

Sorry for nit-picking the code.


More information about the busybox mailing list