Re: [PATCH v2 4/5] block: simplify bdrv_child_user_desc()

2021-05-31 Thread Vladimir Sementsov-Ogievskiy 

31.05.2021 18:50, Kevin Wolf wrote:

Am 04.05.2021 um 11:45 hat Vladimir Sementsov-Ogievskiy geschrieben:

All existing parent types (block nodes, block devices, jobs) has the
realization. So, drop unreachable code.

Signed-off-by: Vladimir Sementsov-Ogievskiy 


Your fixes the other days showed that vvfat has a BdrvChildClass, too.
This instance doesn't define .get_parent_desc(), so the code that this
patch removes is potentially still reachable.



Right. Will fix it and add an assertion to bdrv_attach_child_common()


--
Best regards,
Vladimir

Re: [PATCH v2 4/5] block: simplify bdrv_child_user_desc()

2021-05-31 Thread Kevin Wolf 
Am 04.05.2021 um 11:45 hat Vladimir Sementsov-Ogievskiy geschrieben:
> All existing parent types (block nodes, block devices, jobs) has the
> realization. So, drop unreachable code.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Your fixes the other days showed that vvfat has a BdrvChildClass, too.
This instance doesn't define .get_parent_desc(), so the code that this
patch removes is potentially still reachable.

Kevin

Re: [PATCH v2 4/5] block: simplify bdrv_child_user_desc()

2021-05-10 Thread Vladimir Sementsov-Ogievskiy 

10.05.2021 18:33, Alberto Garcia wrote:

On Tue 04 May 2021 11:45:09 AM CEST, Vladimir Sementsov-Ogievskiy 
 wrote:

All existing parent types (block nodes, block devices, jobs) has the
realization. So, drop unreachable code.


s/has/have/ , and I'm not sure what "have the realization" means


  static char *bdrv_child_user_desc(BdrvChild *c)
  {
-if (c->klass->get_parent_desc) {
-return c->klass->get_parent_desc(c);
-}
-
-return g_strdup("another user");
+return c->klass->get_parent_desc(c);
  }


Should we also assert(c->klass->get_parent_desc) ?



No, as crash on calling zero pointer is practically not worse than crash on 
failed assertion.


--
Best regards,
Vladimir

Re: [PATCH v2 4/5] block: simplify bdrv_child_user_desc()

2021-05-10 Thread Alberto Garcia 
On Tue 04 May 2021 11:45:09 AM CEST, Vladimir Sementsov-Ogievskiy 
 wrote:
> All existing parent types (block nodes, block devices, jobs) has the
> realization. So, drop unreachable code.

s/has/have/ , and I'm not sure what "have the realization" means

>  static char *bdrv_child_user_desc(BdrvChild *c)
>  {
> -if (c->klass->get_parent_desc) {
> -return c->klass->get_parent_desc(c);
> -}
> -
> -return g_strdup("another user");
> +return c->klass->get_parent_desc(c);
>  }

Should we also assert(c->klass->get_parent_desc) ?

Berto