[PATCH 2/3] Discard the legacy interface MEMGETOOBSEL in flash_eraseall

[Joakim Tjernlund]

"stanley.miao" <stanley.miao at windriver.com> wrote on 2010/06/11 12:36:08:
>
> Joakim Tjernlund wrote:
> > Joakim Tjernlund/Transmode wrote on 2010/06/11 11:44:27:
> >
> >>> The "struct nand_oobinfo" is able to record only 32 ECC code positions,which
> >>> is not enough for many big NAND chips. Therefore, this structure is replaced
> >>> by "struct nand_ecclayout" in linux kernel from the version 2.6.17.
> >>> Consequently, the ioctl command changed from MEMGETOOBSEL to ECCGETLAYOUT.
> >>>
> >>> Now update flash_eraseall to use the new ioctl command ECCGETLAYOUT. In order
> >>> to keep compatible with the old linux kernel, a linux version detection
> >>> function is added.
> >>>
> >>> Signed-off-by: Stanley.Miao <stanley.miao at windriver.com>
> >>> ---
> >>>  flash_eraseall.c |   71 ++++++++++++++++++++++++++++++++----------------------
> >>>  1 files changed, 42 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/flash_eraseall.c b/flash_eraseall.c
> >>> index a22fc49..4e2108b 100644
> >>> --- a/flash_eraseall.c
> >>> +++ b/flash_eraseall.c
> >>> @@ -43,6 +43,8 @@
> >>>  #define PROGRAM "flash_eraseall"
> >>>  #define VERSION "$Revision: 1.22 $"
> >>>
> >>> +#define LINUX_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
> >>> +
> >>>  static const char *exe_name;
> >>>  static const char *mtd_device;
> >>>  static int quiet;      /* true -- don't output progress */
> >>> @@ -55,6 +57,21 @@ static void display_version (void);
> >>>  static struct jffs2_unknown_node cleanmarker;
> >>>  int target_endian = __BYTE_ORDER;
> >>>
> >>> +static int get_linux_version(void)
> >>> +{
> >>> +   FILE *fd;
> >>> +   int a = 0, b = 0, c = 0;
> >>> +
> >>> +   fd = fopen("/proc/version", "r");
> >>> +   if (fd) {
> >>> +      fscanf(fd, "Linux version %d.%d.%d", &a, &b, &c);
> >>> +      fclose(fd);
> >>> +   }
> >>> +
> >>> +   return LINUX_VERSION(a, b, c);
> >>> +}
> >>> +
> >>> +
> >>>
> >> The fopen could fail and if it does, I think you should default to "assume
> >> latest kernel"
> >> Same goes for fscanf, you should at least test the return value.
> >>
>

Forgot to say: Hi
:)

> I am not sure what kernel version the proc file "version" began to exist.

But the proc FS might not be mounted.

> If the fopen failed, it must be a old kernel, so I set the default to
> the old kernel.
> It is the same with the current mtd-utils.

I think that behaviour is wrong, one should assume current kernel should
one fail to retrive the exact version since it is much more likly that
the user is running someting new enough.

>
> >
> > Forgot, use uname(2) instead of /proc/version
> >
>
> Why ?

Cleaner, faster, shorter, more portable, less likely to fail.

>
> > Does the above scanf work on "2.6.31-gentoo-r6"?
> >
>
> I didn't do the test on all the platforms. Is there any reason that
> cause it not to work ?

I don't remeber how picky scanf is, will it parse the numbers correctly
when you have non WS trailing chars? In any case you need to check the
return value so you know if it failed.

  Jocke