[LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[Jan Stancek]

Test is using a 'size' based on sizeof(struct sched_attr), but
since c48700d8cdbe ("sched_attr: Do not define for glibc >= 2.41")
that structure can now be defined by glibc which also defines additional
fields.

The kernel only checks if size is smaller than SCHED_ATTR_SIZE_VER0 (48 bytes).
Man page mentions this size directly, so just use a fixed size for the test.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/sched_getattr/sched_getattr02.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
index 5efec2ff5a95..d2099d09f532 100644
--- a/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
+++ b/testcases/kernel/syscalls/sched_getattr/sched_getattr02.c
@@ -51,7 +51,7 @@ static struct test_case {
 } test_cases[] = {
 	{&unused_pid, &attr_copy, sizeof(struct sched_attr), 0, ESRCH},
 	{&pid, NULL, sizeof(struct sched_attr), 0, EINVAL},
-	{&pid, &attr_copy, sizeof(struct sched_attr) - 1, 0, EINVAL},
+	{&pid, &attr_copy, 47, 0, EINVAL},
 	{&pid, &attr_copy, sizeof(struct sched_attr), 1000, EINVAL}
 };
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[Li Wang]

Jan Stancek <jstancek@redhat.com> wrote:


> -       {&pid, &attr_copy, sizeof(struct sched_attr) - 1, 0, EINVAL},
> +       {&pid, &attr_copy, 47, 0, EINVAL},
>

"EINVAL size is invalid; that is, it is smaller than the initial version of
the sched_attr structure (48 bytes) or larger than the system page size."

Or we could add one more item for testing 'page_size + 1'.

Anyway, this workaround looks good.
Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[Petr Vorel]

Hi Jan, Li,

> Jan Stancek <jstancek@redhat.com> wrote:


> > -       {&pid, &attr_copy, sizeof(struct sched_attr) - 1, 0, EINVAL},
> > +       {&pid, &attr_copy, 47, 0, EINVAL},

LGTM. But using "SCHED_ATTR_SIZE_VER0 - 1" would be more descriptive than magic
number 47. But for that we would need to add a fallaback definition to
include/lapi/sched.h. But sure, it's described in the commit message, thus
documented.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> "EINVAL size is invalid; that is, it is smaller than the initial version of
> the sched_attr structure (48 bytes) or larger than the system page size."

> Or we could add one more item for testing 'page_size + 1'.

+1 (having to tests).

Kind regards,
Petr

> Anyway, this workaround looks good.
> Reviewed-by: Li Wang <liwang@redhat.com>

+++ include/lapi/sched.h
@@ -46,6 +46,8 @@ static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
 {
 	return syscall(__NR_sched_getattr, pid, attr, size, flags);
 }
+#else
+# define SCHED_ATTR_SIZE_VER0	48	/* sizeof first published struct */
 #endif
 
 #ifndef HAVE_CLONE3

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[Jan Stancek]

On Mon, Nov 4, 2024 at 10:13 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan, Li,
>
> > Jan Stancek <jstancek@redhat.com> wrote:
>
>
> > > -       {&pid, &attr_copy, sizeof(struct sched_attr) - 1, 0, EINVAL},
> > > +       {&pid, &attr_copy, 47, 0, EINVAL},
>
> LGTM. But using "SCHED_ATTR_SIZE_VER0 - 1" would be more descriptive than magic
> number 47. But for that we would need to add a fallaback definition to
> include/lapi/sched.h. But sure, it's described in the commit message, thus
> documented.

OK, pushed with the usage of define SCHED_ATTR_SIZE_VER0.

>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > "EINVAL size is invalid; that is, it is smaller than the initial version of
> > the sched_attr structure (48 bytes) or larger than the system page size."
>
> > Or we could add one more item for testing 'page_size + 1'.
>
> +1 (having to tests).
>
> Kind regards,
> Petr
>
> > Anyway, this workaround looks good.
> > Reviewed-by: Li Wang <liwang@redhat.com>
>
> +++ include/lapi/sched.h
> @@ -46,6 +46,8 @@ static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
>  {
>         return syscall(__NR_sched_getattr, pid, attr, size, flags);
>  }
> +#else
> +# define SCHED_ATTR_SIZE_VER0  48      /* sizeof first published struct */

^^ not in else branch, the check is "ifndef", so that's where it needs
to be defined


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/sched_getattr02: use a fixed size for EINVAL test

[Petr Vorel]

Hi Jan,

...
> > +++ include/lapi/sched.h
> > @@ -46,6 +46,8 @@ static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
> >  {
> >         return syscall(__NR_sched_getattr, pid, attr, size, flags);
> >  }
> > +#else
> > +# define SCHED_ATTR_SIZE_VER0  48      /* sizeof first published struct */

> ^^ not in else branch, the check is "ifndef", so that's where it needs
> to be defined

I'm sorry, thanks for catching my error.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp