diff mbox series

[1/2] cgroup: Fix build with -Werror=return-type

Message ID 20200622074314.22098-1-petr.vorel@suse.com
State Accepted
Headers show
Series [1/2] cgroup: Fix build with -Werror=return-type | expand

Commit Message

Petr Vorel June 22, 2020, 7:43 a.m. UTC 
From: Petr Vorel <pvorel@suse.cz>

Adding bogus return to keep compiler happy.

Fixes: 3b716981b ("lib: add new cgroup test API")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

IMHO using pragma would be better, but I don't know how to write it portable way.

Kind regards,
Petr

 lib/tst_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Li Wang June 22, 2020, 9:08 a.m. UTC | #1 
On Mon, Jun 22, 2020 at 3:43 PM Petr Vorel <petr.vorel@suse.com> wrote:

> From: Petr Vorel <pvorel@suse.cz>
>
> Adding bogus return to keep compiler happy.
>
> Fixes: 3b716981b ("lib: add new cgroup test API")
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi,
>
> IMHO using pragma would be better, but I don't know how to write it
> portable way.
>
> Kind regards,
> Petr
>
>  lib/tst_cgroup.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index f55d8818d..0118dd7b2 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -49,6 +49,7 @@ enum tst_cgroup_ver tst_cgroup_version(void)
>                 return TST_CGROUP_V1;
>
>         tst_brk(TCONF, "Cgroup is not configured");
> +       return TST_CGROUP_V1; /* fix -Werror=return-type */
>

This return looks strange since it will never go to here.

How about this?

enum tst_cgroup_ver tst_cgroup_version(void)
{
        enum tst_cgroup_ver cg_ver;

        if (tst_cgroup_check("cgroup2")) {
                if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
                        cg_ver = TST_CGROUP_V1;
                else
                        cg_ver = TST_CGROUP_V2;

                goto out;
        }

        if (tst_cgroup_check("cgroup"))
                cg_ver = TST_CGROUP_V1;

        if (!tg_ver)
                tst_brk(TCONF, "Cgroup is not configured");

out:
        return cg_ver;
}
Petr Vorel June 22, 2020, 9:22 a.m. UTC | #2 
Hi Li,

> >         tst_brk(TCONF, "Cgroup is not configured");
> > +       return TST_CGROUP_V1; /* fix -Werror=return-type */


> This return looks strange since it will never go to here.

> How about this?

I'm sorry, I overlooked your mail and push whole patchset.

LGTM, feel free to merge.
Tested-by: Petr Vorel <pvorel@suse.cz>

> enum tst_cgroup_ver tst_cgroup_version(void)
> {
>         enum tst_cgroup_ver cg_ver;

>         if (tst_cgroup_check("cgroup2")) {
>                 if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
>                         cg_ver = TST_CGROUP_V1;
>                 else
>                         cg_ver = TST_CGROUP_V2;

>                 goto out;
>         }

>         if (tst_cgroup_check("cgroup"))
>                 cg_ver = TST_CGROUP_V1;

>         if (!tg_ver)
Typo:
         if (!cg_ver)
>                 tst_brk(TCONF, "Cgroup is not configured");

> out:
>         return cg_ver;
> }

Kind regards,
Petr
Li Wang June 22, 2020, 10:06 a.m. UTC | #3 
On Mon, Jun 22, 2020 at 5:22 PM Petr Vorel <pvorel@suse.com> wrote:

> Hi Li,
>
> > >         tst_brk(TCONF, "Cgroup is not configured");
> > > +       return TST_CGROUP_V1; /* fix -Werror=return-type */
>
>
> > This return looks strange since it will never go to here.
>
> > How about this?
>
> I'm sorry, I overlooked your mail and push whole patchset.
>

Never mind:).

I resend a patch v2 to cover more situations, that would be great if you
can help review/test again.
Li Wang June 22, 2020, 11:40 a.m. UTC | #4 
On Mon, Jun 22, 2020 at 5:22 PM Petr Vorel <pvorel@suse.com> wrote:

> Hi Li,
>
> > >         tst_brk(TCONF, "Cgroup is not configured");
> > > +       return TST_CGROUP_V1; /* fix -Werror=return-type */
>
>
> > This return looks strange since it will never go to here.
>
> > How about this?
>
> I'm sorry, I overlooked your mail and push whole patchset.
>
> LGTM, feel free to merge.
> Tested-by: Petr Vorel <pvorel@suse.cz>
>

This is correct, I go back to this v1 way and pushed. Thanks Petr!
diff mbox series

Patch

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index f55d8818d..0118dd7b2 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -49,6 +49,7 @@  enum tst_cgroup_ver tst_cgroup_version(void)
 		return TST_CGROUP_V1;
 
 	tst_brk(TCONF, "Cgroup is not configured");
+	return TST_CGROUP_V1; /* fix -Werror=return-type */
 }
 
 static void tst_cgroup1_mount(const char *name, const char *option,