| Message ID | 1535013819-1646-1-git-send-email-huijin.park@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: change len type from signed to unsigned type | expand |
Hi Huijin, On Thu, 23 Aug 2018 04:43:39 -0400 Huijin Park <huijin.park@samsung.com> wrote: > From: "huijin.park" <huijin.park@samsung.com> > > assign of a signed value which has type 'int' to a variable of > a bigger unsigned integer type 'uint64_t'. Why are you mentioning u64? AFAICT, the len passed to erase_write() is always an unsigned int. > this is ok most of the time, but can lead to unexpectedly large > resulting value if the original signed value is negative. > in addtion, the callers of the erase_write() pass the len parameter ^In addition, > as unsigned type. > > Signed-off-by: huijin.park <huijin.park@samsung.com> > --- > drivers/mtd/mtdblock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c > index a5b1933..b2d5ed1 100644 > --- a/drivers/mtd/mtdblock.c > +++ b/drivers/mtd/mtdblock.c > @@ -56,7 +56,7 @@ struct mtdblk_dev { > */ > > static int erase_write (struct mtd_info *mtd, unsigned long pos, > - int len, const char *buf) > + unsigned int len, const char *buf) The diff looks good, but the commit message is not clear at all. Can you reword it? Thanks, Boris > { > struct erase_info erase; > size_t retlen;
Hi Boris, On Wed, Oct 31, 2018 at 11:02 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > Hi Huijin, > > On Thu, 23 Aug 2018 04:43:39 -0400 > Huijin Park <huijin.park@samsung.com> wrote: > > > From: "huijin.park" <huijin.park@samsung.com> > > > > assign of a signed value which has type 'int' to a variable of > > a bigger unsigned integer type 'uint64_t'. > > Why are you mentioning u64? AFAICT, the len passed to erase_write() is > always an unsigned int. It's my mistake. Messages about u64 are not related to this patch. > > > this is ok most of the time, but can lead to unexpectedly large > > resulting value if the original signed value is negative. > > in addtion, the callers of the erase_write() pass the len parameter > > ^In addition, > > > as unsigned type. > > > > Signed-off-by: huijin.park <huijin.park@samsung.com> > > --- > > drivers/mtd/mtdblock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c > > index a5b1933..b2d5ed1 100644 > > --- a/drivers/mtd/mtdblock.c > > +++ b/drivers/mtd/mtdblock.c > > @@ -56,7 +56,7 @@ struct mtdblk_dev { > > */ > > > > static int erase_write (struct mtd_info *mtd, unsigned long pos, > > - int len, const char *buf) > > + unsigned int len, const char *buf) > > The diff looks good, but the commit message is not clear at all. Can > you reword it? > > Thanks, > > Boris > > > { > > struct erase_info erase; > > size_t retlen; > I will send again patch after reword the commit message. Thanks & best regards, Huijin
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c index a5b1933..b2d5ed1 100644 --- a/drivers/mtd/mtdblock.c +++ b/drivers/mtd/mtdblock.c @@ -56,7 +56,7 @@ struct mtdblk_dev { */ static int erase_write (struct mtd_info *mtd, unsigned long pos, - int len, const char *buf) + unsigned int len, const char *buf) { struct erase_info erase; size_t retlen;