[PATCH 40/47] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

From: Markus Elfring Date: Mon, 12 Sep 2016 20:00:08 +0200 The rbd_dev_destroy() function was called in two cases by the rbd_dev_probe_parent() function during error handling even if the passed variable contained a null pointer. * Adjust jump targets according to

[PATCH 40/47] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

From: Markus Elfring Date: Mon, 12 Sep 2016 20:00:08 +0200 The rbd_dev_destroy() function was called in two cases by the rbd_dev_probe_parent() function during error handling even if the passed variable contained a null pointer. * Adjust jump targets according to the current Linux coding

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> * Why was the function "rbd_dev_probe_parent" implemented in the way >> that it relies on a sanity check in the function "rbd_dev_destroy" then? > > Because it's not a bad thing? There are different opinions about this implementation detail. > What's wrong with an init to NULL, a possible

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Thu, Nov 26, 2015 at 8:54 AM, SF Markus Elfring wrote: >>> I interpreted the eventual passing of a null pointer to the >>> rbd_dev_destroy() >>> function as an indication for further source code adjustments. >> >> If all error paths could be adjusted so that NULL pointers are never passed >>

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Thu, Nov 26, 2015 at 8:54 AM, SF Markus Elfring wrote: >>> I interpreted the eventual passing of a null pointer to the >>> rbd_dev_destroy() >>> function as an indication for further source code adjustments. >> >> If all error paths could be adjusted so that

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> * Why was the function "rbd_dev_probe_parent" implemented in the way >> that it relies on a sanity check in the function "rbd_dev_destroy" then? > > Because it's not a bad thing? There are different opinions about this implementation detail. > What's wrong with an init to NULL, a possible

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> I interpreted the eventual passing of a null pointer to the rbd_dev_destroy() >> function as an indication for further source code adjustments. > > If all error paths could be adjusted so that NULL pointers are never passed > in, > destroy functions wouldn't need to have a NULL check, would

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

Ah... I see now. Thanks. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Wed, Nov 25, 2015 at 12:55 PM, Dan Carpenter wrote: > On Tue, Nov 24, 2015 at 09:21:06PM +0100, Ilya Dryomov wrote: >> >> Cleanup here is (and should be) done in reverse order. >> > > > Yes. This is true. > >> > I have got an other impression about the appropriate order for the >> >

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 09:21:06PM +0100, Ilya Dryomov wrote: > >> Cleanup here is (and should be) done in reverse order. > > Yes. This is true. > > I have got an other impression about the appropriate order for the > > corresponding > > clean-up function calls. > > > > > >> We allocate parent

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

> "one err bug" as per CodingStyle is a NULL deref on line 2 if foo is NULL. > If it was just "err: kfree(foo); return ret;", a NULL foo would be perfectly > OK. Would it make sense to rename such an issue as the "one error jump label bug"?

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

Ah... I see now. Thanks. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

> "one err bug" as per CodingStyle is a NULL deref on line 2 if foo is NULL. > If it was just "err: kfree(foo); return ret;", a NULL foo would be perfectly > OK. Would it make sense to rename such an issue as the "one error jump label bug"?

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 09:21:06PM +0100, Ilya Dryomov wrote: > >> Cleanup here is (and should be) done in reverse order. > > Yes. This is true. > > I have got an other impression about the appropriate order for the > > corresponding > > clean-up function calls. > > > > > >> We allocate parent

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Wed, Nov 25, 2015 at 12:55 PM, Dan Carpenter wrote: > On Tue, Nov 24, 2015 at 09:21:06PM +0100, Ilya Dryomov wrote: >> >> Cleanup here is (and should be) done in reverse order. >> > > > Yes. This is true. > >> > I have got an other impression about the appropriate

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> I interpreted the eventual passing of a null pointer to the rbd_dev_destroy() >> function as an indication for further source code adjustments. > > If all error paths could be adjusted so that NULL pointers are never passed > in, > destroy functions wouldn't need to have a NULL check, would

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 9:34 PM, SF Markus Elfring wrote: >> Well, there isn't any _literal_ linking (e.g. adding to a link list, >> etc) in this case. We just bump some refs and do probe to fill in the >> newly allocated parent. > > Thanks for your clarification. > > >> The actual linking

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

> Well, there isn't any _literal_ linking (e.g. adding to a link list, > etc) in this case. We just bump some refs and do probe to fill in the > newly allocated parent. Thanks for your clarification. > The actual linking (rbd_dev->parent = parent) is done right before > returning so we never

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 8:23 PM, SF Markus Elfring wrote: >>> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device >>> *rbd_dev, int depth) >>> if (++depth > RBD_MAX_PARENT_CHAIN_LEN) { >>> pr_info("parent chain is too long (%d)\n", depth); >>>

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device >> *rbd_dev, int depth) >> if (++depth > RBD_MAX_PARENT_CHAIN_LEN) { >> pr_info("parent chain is too long (%d)\n", depth); >> ret = -EINVAL; >> - goto out_err; >> +

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Mon, Nov 23, 2015 at 8:46 PM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 23 Nov 2015 20:22:41 +0100 > > The rbd_dev_destroy() function was called in two cases by the > rbd_dev_probe_parent() function during error handling even if > the passed variable contained a null

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 9:34 PM, SF Markus Elfring wrote: >> Well, there isn't any _literal_ linking (e.g. adding to a link list, >> etc) in this case. We just bump some refs and do probe to fill in the >> newly allocated parent. > > Thanks for your clarification.

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Tue, Nov 24, 2015 at 8:23 PM, SF Markus Elfring wrote: >>> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device >>> *rbd_dev, int depth) >>> if (++depth > RBD_MAX_PARENT_CHAIN_LEN) { >>> pr_info("parent chain is too

Re: block-rbd: One function call less in rbd_dev_probe_parent() after error detection

> Well, there isn't any _literal_ linking (e.g. adding to a link list, > etc) in this case. We just bump some refs and do probe to fill in the > newly allocated parent. Thanks for your clarification. > The actual linking (rbd_dev->parent = parent) is done right before > returning so we never

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

>> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device >> *rbd_dev, int depth) >> if (++depth > RBD_MAX_PARENT_CHAIN_LEN) { >> pr_info("parent chain is too long (%d)\n", depth); >> ret = -EINVAL; >> - goto out_err; >> +

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

On Mon, Nov 23, 2015 at 8:46 PM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 23 Nov 2015 20:22:41 +0100 > > The rbd_dev_destroy() function was called in two cases by the > rbd_dev_probe_parent() function during error

[PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

From: Markus Elfring Date: Mon, 23 Nov 2015 20:22:41 +0100 The rbd_dev_destroy() function was called in two cases by the rbd_dev_probe_parent() function during error handling even if the passed variable contained a null pointer. * This implementation detail could be improved by adjustments

[PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

From: Markus Elfring Date: Mon, 23 Nov 2015 20:22:41 +0100 The rbd_dev_destroy() function was called in two cases by the rbd_dev_probe_parent() function during error handling even if the passed variable contained a null pointer. * This implementation detail could