diff mbox series

[10/33] mtd: Rename free() to rfree()

Message ID 20200112120216.10.Ifb04203c2bf46c05eaf0015b1140a2ae4bb3e090@changeid
State Superseded
Delegated to: Simon Glass
Headers show
Series sandbox: Move to SDL2 | expand

Commit Message

Simon Glass Jan. 12, 2020, 7:06 p.m. UTC
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(-)

Comments

Masahiro Yamada Feb. 12, 2020, 1:13 p.m. UTC | #1
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
>
Simon Glass Feb. 12, 2020, 5:14 p.m. UTC | #2
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
Simon Goldschmidt Feb. 13, 2020, 9:05 a.m. UTC | #3
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
Simon Glass Feb. 14, 2020, 7:16 p.m. UTC | #4
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 mbox series

Patch

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);
 };
 
 /*