From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga03-in.huawei.com ([119.145.14.66]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bzMP5-0005hf-6q for linux-mtd@lists.infradead.org; Wed, 26 Oct 2016 11:29:56 +0000 Message-ID: <58109360.1050202@huawei.com> Date: Wed, 26 Oct 2016 19:28:33 +0800 From: Sheng Yong MIME-Version: 1.0 To: Dan Carpenter , Boris Brezillon CC: Subject: Re: [bug report] UBI: Fastmap: Do not add vol if it already exists References: <20161025204607.GA27826@elgon.mountain> <58100B0B.3030906@huawei.com> <20161026082536.GP4418@mwanda> In-Reply-To: <20161026082536.GP4418@mwanda> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 10/26/2016 4:25 PM, Dan Carpenter wrote: > On Wed, Oct 26, 2016 at 09:46:51AM +0800, Sheng Yong wrote: >> Hi Dan, >> >> On 10/26/2016 4:46 AM, Dan Carpenter wrote: >>> Hello shengyong, >>> >>> The patch e96a8a3bb671: "UBI: Fastmap: Do not add vol if it already >>> exists" from May 26, 2015, leads to the following static checker >>> warning: >>> >>> drivers/mtd/ubi/fastmap.c:712 ubi_attach_fastmap() >>> warn: PTR_ERR(av) is never (-22) >>> >>> drivers/mtd/ubi/fastmap.c >>> 703 >>> 704 av = add_vol(ai, be32_to_cpu(fmvhdr->vol_id), >>> 705 be32_to_cpu(fmvhdr->used_ebs), >>> 706 be32_to_cpu(fmvhdr->data_pad), >>> 707 fmvhdr->vol_type, >>> 708 be32_to_cpu(fmvhdr->last_eb_bytes)); >>> 709 >>> 710 if (!av) >>> 711 goto fail_bad; >>> 712 if (PTR_ERR(av) == -EINVAL) { >>> >>> av is either -EEXIST or -ENOMEM. It's never -EINVAL. >> The commit e96a8a3bb671 ("UBI: Fastmap: Do not add vol if it already exists") >> adds a "return ERR_PTR(-EINVAL);" to add_vol(). So I think av could be -EINVAL. >> You mean add_vol should return -EEXIST instead of -EINVAL? >> > > Oh, ah. I'm on linux-next. Commit de4c455b3e9f ("UBI: factorize code > used to manipulate volumes at attach time") removed the -EINVAL. Sorry, my bad. I didn't check linux-next. You're right, the caller of add_vol should have checked -EEXIST. And Boris's patch could fix it. Thanks Dan for pointing that out. thanks, Sheng > > It's really Boris to blame for this warning. I'm not sure what the fix > is. > > regards, > dan carpenter > > > . >