Message ID | 20200112120216.10.Ifb04203c2bf46c05eaf0015b1140a2ae4bb3e090@changeid |
---|---|
State | Superseded |
Delegated to: | Simon Glass |
Headers | show |
Series | sandbox: Move to SDL2 | expand |
On Mon, Jan 13, 2020 at 4:08 AM Simon Glass <sjg@chromium.org> wrote: > > This function name conflicts with our desire to #define free() to > something else on sandbox. Since it deals with resources, rename it to > rfree(). > > Signed-off-by: Simon Glass <sjg@chromium.org> I noticed this commit was merged recently. Now 'free' is a reserved keyword you cannot use in U-Boot. Commit cc92c3c thru cf23c7c are horrible. Commit cfda60f should have used 'static inline' instead of #define. I cannot believe it. > --- > > drivers/mtd/mtdcore.c | 4 ++-- > drivers/mtd/nand/raw/denali.c | 2 +- > drivers/mtd/nand/spi/core.c | 2 +- > drivers/mtd/nand/spi/gigadevice.c | 2 +- > drivers/mtd/nand/spi/macronix.c | 2 +- > drivers/mtd/nand/spi/micron.c | 2 +- > drivers/mtd/nand/spi/winbond.c | 2 +- > include/linux/mtd/mtd.h | 4 ++-- > 8 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index dd04d676d5..838c288318 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -1179,10 +1179,10 @@ int mtd_ooblayout_free(struct mtd_info *mtd, int section, > if (!mtd || section < 0) > return -EINVAL; > > - if (!mtd->ooblayout || !mtd->ooblayout->free) > + if (!mtd->ooblayout || !mtd->ooblayout->rfree) > return -ENOTSUPP; > > - return mtd->ooblayout->free(mtd, section, oobfree); > + return mtd->ooblayout->rfree(mtd, section, oobfree); > } > EXPORT_SYMBOL_GPL(mtd_ooblayout_free); > > diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c > index 0a7ca8a8df..f0b528485c 100644 > --- a/drivers/mtd/nand/raw/denali.c > +++ b/drivers/mtd/nand/raw/denali.c > @@ -1178,7 +1178,7 @@ static int denali_ooblayout_free(struct mtd_info *mtd, int section, > > static const struct mtd_ooblayout_ops denali_ooblayout_ops = { > .ecc = denali_ooblayout_ecc, > - .free = denali_ooblayout_free, > + .rfree = denali_ooblayout_free, > }; > > static int denali_multidev_fixup(struct denali_nand_info *denali) > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index cb8ffa3fa9..fba8cc056a 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -1021,7 +1021,7 @@ static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section, > > static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = { > .ecc = spinand_noecc_ooblayout_ecc, > - .free = spinand_noecc_ooblayout_free, > + .rfree = spinand_noecc_ooblayout_free, > }; > > static int spinand_init(struct spinand_device *spinand) > diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c > index 3681c5eed9..e329c3cfc0 100644 > --- a/drivers/mtd/nand/spi/gigadevice.c > +++ b/drivers/mtd/nand/spi/gigadevice.c > @@ -103,7 +103,7 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand, > > static const struct mtd_ooblayout_ops gd5fxgq4xexxg_ooblayout = { > .ecc = gd5fxgq4xexxg_ooblayout_ecc, > - .free = gd5fxgq4xexxg_ooblayout_free, > + .rfree = gd5fxgq4xexxg_ooblayout_free, > }; > > static const struct spinand_info gigadevice_spinand_table[] = { > diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c > index 662c561e50..1119677f6f 100644 > --- a/drivers/mtd/nand/spi/macronix.c > +++ b/drivers/mtd/nand/spi/macronix.c > @@ -47,7 +47,7 @@ static int mx35lfxge4ab_ooblayout_free(struct mtd_info *mtd, int section, > > static const struct mtd_ooblayout_ops mx35lfxge4ab_ooblayout = { > .ecc = mx35lfxge4ab_ooblayout_ecc, > - .free = mx35lfxge4ab_ooblayout_free, > + .rfree = mx35lfxge4ab_ooblayout_free, > }; > > static int mx35lf1ge4ab_get_eccsr(struct spinand_device *spinand, u8 *eccsr) > diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c > index 83951c5d0f..9c24542f96 100644 > --- a/drivers/mtd/nand/spi/micron.c > +++ b/drivers/mtd/nand/spi/micron.c > @@ -63,7 +63,7 @@ static int mt29f2g01abagd_ooblayout_free(struct mtd_info *mtd, int section, > > static const struct mtd_ooblayout_ops mt29f2g01abagd_ooblayout = { > .ecc = mt29f2g01abagd_ooblayout_ecc, > - .free = mt29f2g01abagd_ooblayout_free, > + .rfree = mt29f2g01abagd_ooblayout_free, > }; > > static int mt29f2g01abagd_ecc_get_status(struct spinand_device *spinand, > diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c > index eac811d97c..f3446e71b9 100644 > --- a/drivers/mtd/nand/spi/winbond.c > +++ b/drivers/mtd/nand/spi/winbond.c > @@ -59,7 +59,7 @@ static int w25m02gv_ooblayout_free(struct mtd_info *mtd, int section, > > static const struct mtd_ooblayout_ops w25m02gv_ooblayout = { > .ecc = w25m02gv_ooblayout_ecc, > - .free = w25m02gv_ooblayout_free, > + .rfree = w25m02gv_ooblayout_free, > }; > > static int w25m02gv_select_target(struct spinand_device *spinand, > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index ceffd994de..1b9151714c 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -129,8 +129,8 @@ struct mtd_oob_region { > struct mtd_ooblayout_ops { > int (*ecc)(struct mtd_info *mtd, int section, > struct mtd_oob_region *oobecc); > - int (*free)(struct mtd_info *mtd, int section, > - struct mtd_oob_region *oobfree); > + int (*rfree)(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobfree); > }; > > /* > -- > 2.25.0.rc1.283.g88dfdc4193-goog >
Hi Masahiro, On Wed, 12 Feb 2020 at 06:14, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Mon, Jan 13, 2020 at 4:08 AM Simon Glass <sjg@chromium.org> wrote: > > > > This function name conflicts with our desire to #define free() to > > something else on sandbox. Since it deals with resources, rename it to > > rfree(). > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > I noticed this commit was merged recently. > > Now 'free' is a reserved keyword > you cannot use in U-Boot. > > > Commit cc92c3c thru cf23c7c are horrible. > > > Commit cfda60f should have used > 'static inline' instead of #define. > > I cannot believe it. Are you sure you understand the problem I was trying to solve? I am using dlmalloc's existing means of adding a prefix, but I'm sure we could change it to another way. If we define malloc() as dlmalloc() in dlmalloc.c, then we could add a declaration in dlmalloc.h that uses static inline to convert calls to malloc() to call dlmalloc(). Then anything that doesn't include malloc.h would still call the C library malloc(). Is that what you are thinking? I did look at using a link script instead but it is pretty messy. What do you mean by 'free' being a reserved keyword? Where? So is 'rfree' a good substitute or do you suggest something else? Regards, Simon
On Wed, Feb 12, 2020 at 6:14 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Masahiro, > > On Wed, 12 Feb 2020 at 06:14, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Mon, Jan 13, 2020 at 4:08 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > This function name conflicts with our desire to #define free() to > > > something else on sandbox. Since it deals with resources, rename it to > > > rfree(). > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > I noticed this commit was merged recently. > > > > Now 'free' is a reserved keyword > > you cannot use in U-Boot. > > > > > > Commit cc92c3c thru cf23c7c are horrible. > > > > > > Commit cfda60f should have used > > 'static inline' instead of #define. > > > > I cannot believe it. > > Are you sure you understand the problem I was trying to solve? I am > using dlmalloc's existing means of adding a prefix, but I'm sure we > could change it to another way. > > If we define malloc() as dlmalloc() in dlmalloc.c, then we could add a > declaration in dlmalloc.h that uses static inline to convert calls to > malloc() to call dlmalloc(). Then anything that doesn't include > malloc.h would still call the C library malloc(). Is that what you are > thinking? There is no "malloc()" in dlmalloc.c. It is called "mALLOc()" and by defining USE_DL_PREFIX, you already have converted that to be linked as "dlmalloc()". I think there should be no difference in who calls what when converting your defines to static inline functions. And yes, I also dislike the other patches that remove all occurrences of 'free'. I think without knowing the backgrounds of your patches, that just looks strange. Regards, Simon > > I did look at using a link script instead but it is pretty messy. > > What do you mean by 'free' being a reserved keyword? Where? So is > 'rfree' a good substitute or do you suggest something else? > > Regards, > Simon
Hi Simon, On Thu, 13 Feb 2020 at 02:06, Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > On Wed, Feb 12, 2020 at 6:14 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Masahiro, > > > > On Wed, 12 Feb 2020 at 06:14, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Mon, Jan 13, 2020 at 4:08 AM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > This function name conflicts with our desire to #define free() to > > > > something else on sandbox. Since it deals with resources, rename it to > > > > rfree(). > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > I noticed this commit was merged recently. > > > > > > Now 'free' is a reserved keyword > > > you cannot use in U-Boot. > > > > > > > > > Commit cc92c3c thru cf23c7c are horrible. > > > > > > > > > Commit cfda60f should have used > > > 'static inline' instead of #define. > > > > > > I cannot believe it. > > > > Are you sure you understand the problem I was trying to solve? I am > > using dlmalloc's existing means of adding a prefix, but I'm sure we > > could change it to another way. > > > > If we define malloc() as dlmalloc() in dlmalloc.c, then we could add a > > declaration in dlmalloc.h that uses static inline to convert calls to > > malloc() to call dlmalloc(). Then anything that doesn't include > > malloc.h would still call the C library malloc(). Is that what you are > > thinking? > > There is no "malloc()" in dlmalloc.c. It is called "mALLOc()" and by defining > USE_DL_PREFIX, you already have converted that to be linked as "dlmalloc()". > > I think there should be no difference in who calls what when converting your > defines to static inline functions. > > And yes, I also dislike the other patches that remove all occurrences of > 'free'. I think without knowing the backgrounds of your patches, that just > looks strange. OK I think that will work. I was trying to use what is there already, but it is a bit ugly. Regards, Simon
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index dd04d676d5..838c288318 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1179,10 +1179,10 @@ int mtd_ooblayout_free(struct mtd_info *mtd, int section, if (!mtd || section < 0) return -EINVAL; - if (!mtd->ooblayout || !mtd->ooblayout->free) + if (!mtd->ooblayout || !mtd->ooblayout->rfree) return -ENOTSUPP; - return mtd->ooblayout->free(mtd, section, oobfree); + return mtd->ooblayout->rfree(mtd, section, oobfree); } EXPORT_SYMBOL_GPL(mtd_ooblayout_free); diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 0a7ca8a8df..f0b528485c 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -1178,7 +1178,7 @@ static int denali_ooblayout_free(struct mtd_info *mtd, int section, static const struct mtd_ooblayout_ops denali_ooblayout_ops = { .ecc = denali_ooblayout_ecc, - .free = denali_ooblayout_free, + .rfree = denali_ooblayout_free, }; static int denali_multidev_fixup(struct denali_nand_info *denali) diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index cb8ffa3fa9..fba8cc056a 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -1021,7 +1021,7 @@ static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section, static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = { .ecc = spinand_noecc_ooblayout_ecc, - .free = spinand_noecc_ooblayout_free, + .rfree = spinand_noecc_ooblayout_free, }; static int spinand_init(struct spinand_device *spinand) diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c index 3681c5eed9..e329c3cfc0 100644 --- a/drivers/mtd/nand/spi/gigadevice.c +++ b/drivers/mtd/nand/spi/gigadevice.c @@ -103,7 +103,7 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand, static const struct mtd_ooblayout_ops gd5fxgq4xexxg_ooblayout = { .ecc = gd5fxgq4xexxg_ooblayout_ecc, - .free = gd5fxgq4xexxg_ooblayout_free, + .rfree = gd5fxgq4xexxg_ooblayout_free, }; static const struct spinand_info gigadevice_spinand_table[] = { diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c index 662c561e50..1119677f6f 100644 --- a/drivers/mtd/nand/spi/macronix.c +++ b/drivers/mtd/nand/spi/macronix.c @@ -47,7 +47,7 @@ static int mx35lfxge4ab_ooblayout_free(struct mtd_info *mtd, int section, static const struct mtd_ooblayout_ops mx35lfxge4ab_ooblayout = { .ecc = mx35lfxge4ab_ooblayout_ecc, - .free = mx35lfxge4ab_ooblayout_free, + .rfree = mx35lfxge4ab_ooblayout_free, }; static int mx35lf1ge4ab_get_eccsr(struct spinand_device *spinand, u8 *eccsr) diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c index 83951c5d0f..9c24542f96 100644 --- a/drivers/mtd/nand/spi/micron.c +++ b/drivers/mtd/nand/spi/micron.c @@ -63,7 +63,7 @@ static int mt29f2g01abagd_ooblayout_free(struct mtd_info *mtd, int section, static const struct mtd_ooblayout_ops mt29f2g01abagd_ooblayout = { .ecc = mt29f2g01abagd_ooblayout_ecc, - .free = mt29f2g01abagd_ooblayout_free, + .rfree = mt29f2g01abagd_ooblayout_free, }; static int mt29f2g01abagd_ecc_get_status(struct spinand_device *spinand, diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c index eac811d97c..f3446e71b9 100644 --- a/drivers/mtd/nand/spi/winbond.c +++ b/drivers/mtd/nand/spi/winbond.c @@ -59,7 +59,7 @@ static int w25m02gv_ooblayout_free(struct mtd_info *mtd, int section, static const struct mtd_ooblayout_ops w25m02gv_ooblayout = { .ecc = w25m02gv_ooblayout_ecc, - .free = w25m02gv_ooblayout_free, + .rfree = w25m02gv_ooblayout_free, }; static int w25m02gv_select_target(struct spinand_device *spinand, diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index ceffd994de..1b9151714c 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -129,8 +129,8 @@ struct mtd_oob_region { struct mtd_ooblayout_ops { int (*ecc)(struct mtd_info *mtd, int section, struct mtd_oob_region *oobecc); - int (*free)(struct mtd_info *mtd, int section, - struct mtd_oob_region *oobfree); + int (*rfree)(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobfree); }; /*
This function name conflicts with our desire to #define free() to something else on sandbox. Since it deals with resources, rename it to rfree(). Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/mtd/mtdcore.c | 4 ++-- drivers/mtd/nand/raw/denali.c | 2 +- drivers/mtd/nand/spi/core.c | 2 +- drivers/mtd/nand/spi/gigadevice.c | 2 +- drivers/mtd/nand/spi/macronix.c | 2 +- drivers/mtd/nand/spi/micron.c | 2 +- drivers/mtd/nand/spi/winbond.c | 2 +- include/linux/mtd/mtd.h | 4 ++-- 8 files changed, 10 insertions(+), 10 deletions(-)