mtd-utils: various fixes, apply OpenEmbedded patches

mtd-utils: various fixes, apply OpenEmbedded patches

[David Oberhollenzer]

This patch series mostly consists of minor fixes that have piled
up on my end.

The series mainly includes some of the patches applied localy by
the OpenEmbedded project that haven't been submitted upstream so
far.

Two more patches that OpenEmbedded supposedly applies to recent
mtd-utils have not been included:
 - One that fixes logic in flash_erase that doesn't exist anymore.
 - One that adds an excluion flag to mkfs.jffs2. It doesn't apply
   and is sprinkled with debug prints.


Regards,

David

--
 include/common.h             | 25 ++++++-------------------
 include/libmtd.h             |  3 +--
 jffsX-utils/mkfs.jffs2.c     |  1 +
 lib/libmtd.c                 | 12 ++++++------
 lib/libmtd_int.h             |  1 +
 lib/libmtd_legacy.c          | 18 ++++++++++++++++++
 lib/libubi.c                 |  5 ++++-
 ubi-utils/mtdinfo.c          |  5 +----
 ubi-utils/ubiformat.c        |  2 --
 ubifs-utils/mkfs.ubifs/key.h | 16 ++++++++++------
 10 files changed, 48 insertions(+), 40 deletions(-)

[PATCH 1/5] Fix alignment trap triggered by NEON instructions

[David Oberhollenzer]

From: Yuanjie Huang <Yuanjie.Huang@windriver.com>

NEON instruction VLD1.64 was used to copy 64 bits data after type
casting, and they will trigger alignment trap.
This patch uses memcpy to avoid alignment problem.

Signed-off-by: Yuanjie Huang <Yuanjie.Huang@windriver.com>
Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/key.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/key.h b/ubifs-utils/mkfs.ubifs/key.h
index 39379fd..118858b 100644
--- a/ubifs-utils/mkfs.ubifs/key.h
+++ b/ubifs-utils/mkfs.ubifs/key.h
@@ -159,10 +159,12 @@ static inline void data_key_init(union ubifs_key *key, ino_t inum,
  */
 static inline void key_write(const union ubifs_key *from, void *to)
 {
-	union ubifs_key *t = to;
+	__le32 x[2];
 
-	t->j32[0] = cpu_to_le32(from->u32[0]);
-	t->j32[1] = cpu_to_le32(from->u32[1]);
+	x[0] = cpu_to_le32(from->u32[0]);
+	x[1] = cpu_to_le32(from->u32[1]);
+
+	memcpy(to, &x, 8);
 	memset(to + 8, 0, UBIFS_MAX_KEY_LEN - 8);
 }
 
@@ -174,10 +176,12 @@ static inline void key_write(const union ubifs_key *from, void *to)
  */
 static inline void key_write_idx(const union ubifs_key *from, void *to)
 {
-	union ubifs_key *t = to;
+	__le32 x[2];
+
+	x[0] = cpu_to_le32(from->u32[0]);
+	x[1] = cpu_to_le32(from->u32[1]);
 
-	t->j32[0] = cpu_to_le32(from->u32[0]);
-	t->j32[1] = cpu_to_le32(from->u32[1]);
+	memcpy(to, &x, 8);
 }
 
 /**
-- 
2.10.2

[PATCH 2/5] Replace rpmatch() usage with checking first character of line

[David Oberhollenzer]

This is based on the patch from Khem Raj used by openembedded. In
addition to the original patch, this also removes the fallback
implementation that was provided for C libraries that don't implement
rpmatch.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 include/common.h | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/include/common.h b/include/common.h
index d0c706d..d609257 100644
--- a/include/common.h
+++ b/include/common.h
@@ -129,21 +129,6 @@ extern "C" {
 	fprintf(stderr, "%s: warning!: " fmt "\n", PROGRAM_NAME, ##__VA_ARGS__); \
 } while(0)
 
-/* uClibc versions before 0.9.34 and musl don't have rpmatch() */
-#if defined(__UCLIBC__) && \
-		(__UCLIBC_MAJOR__ == 0 && \
-		(__UCLIBC_MINOR__ < 9 || \
-		(__UCLIBC_MINOR__ == 9 && __UCLIBC_SUBLEVEL__ < 34))) || \
-	!defined(__GLIBC__)
-#undef rpmatch
-#define rpmatch __rpmatch
-static inline int __rpmatch(const char *resp)
-{
-    return (resp[0] == 'y' || resp[0] == 'Y') ? 1 :
-	(resp[0] == 'n' || resp[0] == 'N') ? 0 : -1;
-}
-#endif
-
 /**
  * prompt the user for confirmation
  */
@@ -164,10 +149,12 @@ static inline bool prompt(const char *msg, bool def)
 		}
 
 		if (strcmp("\n", line) != 0) {
-			switch (rpmatch(line)) {
-			case 0: ret = false; break;
-			case 1: ret = true; break;
-			case -1:
+			switch (line[0]) {
+			case 'N':
+			case 'n': ret = false; break;
+			case 'Y':
+			case 'y': ret = true; break;
+			default:
 				puts("unknown response; please try again");
 				continue;
 			}
-- 
2.10.2

[PATCH 3/5] Fix build with musl

[David Oberhollenzer]

From: Khem Raj <raj.khem@gmail.com>

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 jffsX-utils/mkfs.jffs2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/jffsX-utils/mkfs.jffs2.c b/jffsX-utils/mkfs.jffs2.c
index 5446a16..ca5e0d5 100644
--- a/jffsX-utils/mkfs.jffs2.c
+++ b/jffsX-utils/mkfs.jffs2.c
@@ -72,6 +72,7 @@
 #include <byteswap.h>
 #include <crc32.h>
 #include <inttypes.h>
+#include <limits.h>
 
 #include "rbtree.h"
 #include "common.h"
-- 
2.10.2

[PATCH 4/5] Fix libmtd behaviour if MTD is not present on the system

[David Oberhollenzer]

The documentation of libmtd_open says, if it returns NULL and errno is
zero, MTD is not present. However, the current version always returns
a libmtd_t object. The function internally checks, if it can access the
MTD sysfs files and, if not, sets a flag to use the procfs fallback.

This patch adds an additional check to libmtd_open, to test if the
MTD procfs file can be read and fails with errno cleared if it does
not exist.

Furhtermore, mtd_get_info is documented to fail with errno set to ENODEV
if MTD is not present. First of all, this was broken in the original
version. It was implemented to specification for the sysfs code path,
but if MTD is not present, that won't be executed, because of the flag
set by libmtd_open. This makes the check not only redundant, but masks
an actual error (the sysfs paths suddenly not being readable anymore).
The legacy path that was used if the sysfs files are not avaible fails
with ENOENT if it cannot read the procfs file. With the above changes
in addition, we don't have a libmtd_t object if neither sysfs nor
procfs is readable, so this error status no longer makes sense.

This patch removes the documentation on the ENODEV errno, and
makes sure that mtd_get_info always returns with apropriate errno
on failure.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 include/libmtd.h      |  3 +--
 lib/libmtd.c          | 12 ++++++------
 lib/libmtd_int.h      |  1 +
 lib/libmtd_legacy.c   | 18 ++++++++++++++++++
 ubi-utils/mtdinfo.c   |  5 +----
 ubi-utils/ubiformat.c |  2 --
 6 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index f9f3164..db85fb4 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -120,8 +120,7 @@ int mtd_dev_present(libmtd_t desc, int mtd_num);
  * @info: the MTD device information is returned here
  *
  * This function fills the passed @info object with general MTD information and
- * returns %0 in case of success and %-1 in case of failure. If MTD subsystem is
- * not present in the system, errno is set to @ENODEV.
+ * returns %0 in case of success and %-1 in case of failure.
  */
 int mtd_get_info(libmtd_t desc, struct mtd_info *info);
 
diff --git a/lib/libmtd.c b/lib/libmtd.c
index 8bc532f..a50f18a 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -578,6 +578,11 @@ libmtd_t libmtd_open(void)
 		free(lib->sysfs_mtd);
 		free(lib->mtd_name);
 		lib->mtd_name = lib->mtd = lib->sysfs_mtd = NULL;
+
+		if (!legacy_procfs_is_supported()) {
+			free(lib);
+			lib = NULL;
+		}
 		return lib;
 	}
 
@@ -676,13 +681,8 @@ int mtd_get_info(libmtd_t desc, struct mtd_info *info)
 	 * devices are present.
 	 */
 	sysfs_mtd = opendir(lib->sysfs_mtd);
-	if (!sysfs_mtd) {
-		if (errno == ENOENT) {
-			errno = ENODEV;
-			return -1;
-		}
+	if (!sysfs_mtd)
 		return sys_errmsg("cannot open \"%s\"", lib->sysfs_mtd);
-	}
 
 	info->lowest_mtd_num = INT_MAX;
 	while (1) {
diff --git a/lib/libmtd_int.h b/lib/libmtd_int.h
index db2f1cf..03b0863 100644
--- a/lib/libmtd_int.h
+++ b/lib/libmtd_int.h
@@ -98,6 +98,7 @@ struct libmtd
 	unsigned int offs64_ioctls:2;
 };
 
+int legacy_procfs_is_supported(void);
 int legacy_dev_present(int mtd_num);
 int legacy_mtd_get_info(struct mtd_info *info);
 int legacy_get_dev_info(const char *node, struct mtd_dev_info *mtd);
diff --git a/lib/libmtd_legacy.c b/lib/libmtd_legacy.c
index ba8eade..46f51fd 100644
--- a/lib/libmtd_legacy.c
+++ b/lib/libmtd_legacy.c
@@ -146,6 +146,24 @@ static int proc_parse_next(struct proc_parse_info *pi)
 }
 
 /**
+ * legacy_procfs_is_supported - legacy version of 'sysfs_is_supported()'.
+ *
+ * Check if we can access the procfs files for the MTD subsystem.
+ */
+int legacy_procfs_is_supported(void)
+{
+	if (access(MTD_PROC_FILE, R_OK) != 0) {
+		if (errno == ENOENT) {
+			errno = 0;
+		} else {
+			sys_errmsg("cannot read \"%s\"", MTD_PROC_FILE);
+		}
+		return 0;
+	}
+	return 1;
+}
+
+/**
  * legacy_dev_presentl - legacy version of 'mtd_dev_present()'.
  * @info: the MTD device information is returned here
  *
diff --git a/ubi-utils/mtdinfo.c b/ubi-utils/mtdinfo.c
index 11e59c1..0606ab0 100644
--- a/ubi-utils/mtdinfo.c
+++ b/ubi-utils/mtdinfo.c
@@ -407,11 +407,8 @@ int main(int argc, char * const argv[])
 	}
 
 	err = mtd_get_info(libmtd, &mtd_info);
-	if (err) {
-		if (errno == ENODEV)
-			return errmsg("MTD is not present");
+	if (err)
 		return sys_errmsg("cannot get MTD information");
-	}
 
 	if (!args.all && args.node) {
 		int mtdn;
diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
index 68906f2..896fe20 100644
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -698,8 +698,6 @@ int main(int argc, char * const argv[])
 
 	err = mtd_get_info(libmtd, &mtd_info);
 	if (err) {
-		if (errno == ENODEV)
-			errmsg("MTD is not present");
 		sys_errmsg("cannot get MTD information");
 		goto out_close_mtd;
 	}
-- 
2.10.2

[PATCH 5/5] Return correct error number in ubi_get_vol_info1

[David Oberhollenzer]

If the specified UBI device or volume does not exist, the function
is supposed to set errno to ENODEV.

This patch adds a check to ubi_get_vol_info1 to change the errno
to ENODEV if vol_get_major cannot access the underlying sysfs file,
so the function propperly returns that the device or volume does
not exist, instead of failing with errno set to ENOENT.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 lib/libubi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/libubi.c b/lib/libubi.c
index 99022dd..f1cc37a 100644
--- a/lib/libubi.c
+++ b/lib/libubi.c
@@ -1242,8 +1242,11 @@ int ubi_get_vol_info1(libubi_t desc, int dev_num, int vol_id,
 	info->dev_num = dev_num;
 	info->vol_id = vol_id;
 
-	if (vol_get_major(lib, dev_num, vol_id, &info->major, &info->minor))
+	if (vol_get_major(lib, dev_num, vol_id, &info->major, &info->minor)) {
+		if (errno == ENOENT)
+			errno = ENODEV;
 		return -1;
+	}
 
 	ret = vol_read_data(lib->vol_type, dev_num, vol_id, buf, 50);
 	if (ret < 0)
-- 
2.10.2

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[Thomas Petazzoni]

Hello,

On Wed, 22 Mar 2017 11:22:52 +0100, David Oberhollenzer wrote:

> The series mainly includes some of the patches applied localy by
> the OpenEmbedded project that haven't been submitted upstream so
> far.

In Buildroot, we're still using the old 1.5.2 mtd-utils version, but we
have three patches on it, you might be interested in having a look to
see if any of those patches are still relevant or not:

  https://git.buildroot.org/buildroot/tree/package/mtd/

We'll try to move to the 2.x version soon.

Thanks for all this work on mtd-utils, very nice to see the project
more active!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[David Oberhollenzer]

Hi,

Thanks for the pointers. I took a look at the buildroot patches.

The first one (ubinize return status) has been applied shortly after
the mtd-utils 1.5.2 release.

The second patch (about execinfo.h) sparked a discussion with Richard
prefering a libmissing implementation instead. A libmissing has been
added in August last year that provides a dummy implementation of the
backtrace family of functions for libraries like musl that don't
provided them.

The third patch, that removes the bits/stdio_lim.h inclusion was also
applied already between the 1.5.2 and 2.0.0-rc1 releases.


Thanks,

David

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[Thomas Petazzoni]

Hello,

On Thu, 23 Mar 2017 10:33:21 +0100, David Oberhollenzer wrote:

> The first one (ubinize return status) has been applied shortly after
> the mtd-utils 1.5.2 release.
> 
> The second patch (about execinfo.h) sparked a discussion with Richard
> prefering a libmissing implementation instead. A libmissing has been
> added in August last year that provides a dummy implementation of the
> backtrace family of functions for libraries like musl that don't
> provided them.
> 
> The third patch, that removes the bits/stdio_lim.h inclusion was also
> applied already between the 1.5.2 and 2.0.0-rc1 releases.

Awesome, thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[Andrea Adami]

On Thu, Mar 23, 2017 at 10:45 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Thu, 23 Mar 2017 10:33:21 +0100, David Oberhollenzer wrote:
>
>> The first one (ubinize return status) has been applied shortly after
>> the mtd-utils 1.5.2 release.
>>
>> The second patch (about execinfo.h) sparked a discussion with Richard
>> prefering a libmissing implementation instead. A libmissing has been
>> added in August last year that provides a dummy implementation of the
>> backtrace family of functions for libraries like musl that don't
>> provided them.
>>
>> The third patch, that removes the bits/stdio_lim.h inclusion was also
>> applied already between the 1.5.2 and 2.0.0-rc1 releases.
>
> Awesome, thanks a lot!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


Hi,
while you're reviewing old patches please note there are some little
klibc fixes for 1.52 here:

http://cgit.openembedded.org/meta-openembedded/tree/meta-initramfs/recipes-devtools/mtd/ubi-utils-klibc

Probably you could consider the fixes for rpmatch() and getline(). Unsure about
iniparser_getdouble.

Thanks
Andrea

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[David Oberhollenzer]

As there were no complaints about the patches I posted last week, I
applied them to mtd-utils.git.


On 03/23/2017 11:57 AM, Andrea Adami wrote:
> Hi,
> while you're reviewing old patches please note there are some little
> klibc fixes for 1.52 here:
> 
> http://cgit.openembedded.org/meta-openembedded/tree/meta-initramfs/recipes-devtools/mtd/ubi-utils-klibc
> 
> Probably you could consider the fixes for rpmatch() and getline(). Unsure about
> iniparser_getdouble.
> 
> Thanks
> Andrea
> 

I took a look at those. The Makefile patches aren't really relevant
anymore, after the switch to Autotools (also, as noted in the patch,
the first one is very specific for your use case).

The PRIxoff_t/PRIdoff_t patch (number 4) doesn't apply anymore. From a
brief look at the log it appears that the format specifies have been
fixed at least 3 times after the date of your patch. The current
version uses autoconf to determine the sizeof off_t and loff_t. The
header sets the format specifers based on the size only, so this
_should_ make magic libc vendor/feature checks obsolete.

As for the prompt() patch (number 5), the use of rpmatch() has recently
been removed. I would hesitate removing the use of getline() as proposed
in the patch, first of all the proposed patch would alter the behavior
of prompt() if more than one character was entered and second, getline
has actually been accepted into the standard some ~9 years ago.[1]


If there are no objections, I'll apply the libubi ioctl and
libiniparser patches (number 3 and 6 respectively).


Thanks,

David

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getdelim.html

Re: mtd-utils: various fixes, apply OpenEmbedded patches

[Andrea Adami]

On Wed, Mar 29, 2017 at 3:12 PM, David Oberhollenzer
<david.oberhollenzer@sigma-star.at> wrote:
> As there were no complaints about the patches I posted last week, I
> applied them to mtd-utils.git.
>
>
> On 03/23/2017 11:57 AM, Andrea Adami wrote:
>> Hi,
>> while you're reviewing old patches please note there are some little
>> klibc fixes for 1.52 here:
>>
>> http://cgit.openembedded.org/meta-openembedded/tree/meta-initramfs/recipes-devtools/mtd/ubi-utils-klibc
>>
>> Probably you could consider the fixes for rpmatch() and getline(). Unsure about
>> iniparser_getdouble.
>>
>> Thanks
>> Andrea
>>
>
> I took a look at those. The Makefile patches aren't really relevant
> anymore, after the switch to Autotools (also, as noted in the patch,
> the first one is very specific for your use case).
>
> The PRIxoff_t/PRIdoff_t patch (number 4) doesn't apply anymore. From a
> brief look at the log it appears that the format specifies have been
> fixed at least 3 times after the date of your patch. The current
> version uses autoconf to determine the sizeof off_t and loff_t. The
> header sets the format specifers based on the size only, so this
> _should_ make magic libc vendor/feature checks obsolete.
>
> As for the prompt() patch (number 5), the use of rpmatch() has recently
> been removed. I would hesitate removing the use of getline() as proposed
> in the patch, first of all the proposed patch would alter the behavior
> of prompt() if more than one character was entered and second, getline
> has actually been accepted into the standard some ~9 years ago.[1]
>
>
> If there are no objections, I'll apply the libubi ioctl and
> libiniparser patches (number 3 and 6 respectively).
>
>
> Thanks,
>
> David
>
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getdelim.html


David,

thanks for your attention.

I've just pointed these old patches to the attention of the list
because there were other references to mtd 1.5x.
There isn't anything urgent with these klibc specific fixes, just FYI

I'll try to catch up with 2.x, at least the ubi-utils.
For our project we need just kexec-static and for binary size of the
executable klibc is still the best.

To be honest, I have yet to try the static approach with a recent musl
libc like Khem suggested.

Regards
Andrea