On Sat, Dec 07, 2019 at 11:51:38PM +0100, Stefano Brivio wrote: > On Fri, 6 Dec 2019 20:45:17 +0100 > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: [...] > > On Thu, Dec 05, 2019 at 11:43:50PM +0100, Stefano Brivio wrote: [...] > > > If the type is not NFT_DATA_VALUE, I guess we shouldn't pass > > > NFT_DATA_VALUE to nft_data_release() here. > > > > The new nft_setelem_parse_key() function makes sure that the key is > > NFT_DATA_VALUE, otherwise bails out and calls nft_data_release() with > > desc.type. > > > > Then, moving forward in nft_add_set_elem() after the > > nft_setelem_parse_key(), if an error occurs, nft_data_release() can be > > called with NFT_DATA_VALUE, because that was already validated by > > nft_setelem_parse_key(). > > > > > Maybe nft_setelem_parse_key() could clean up after itself on error. > > > > It's doing so already, right? See err2: label. > > Right you are, my bad, I mixed up err2: and err1: in nft_set_delelem() > and then forgot about err2: in nft_setelem_parse_key(). > > Well, on the other hand, 'return err;" and 'goto fail_elem;" would have > been easier to follow, but maybe it's just my taste. :) Feel free to update this patch to use the goto tags you are suggesting. Thanks.
