[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync
According to fsync specification [1] some special files (e.g., a pipe, FIFO, or socket) don't support synchronization and return either EROFS or EINVAL.
On the linux side the sys_fsync -> do_fsync() checks if the requested file has f_op->fsync defined. If not it returns EINVAL [2].
This commit prevents writing error messages for files (devices), which do not support fsync().
[1] - http://man7.org/linux/man-pages/man2/fsync.2.html [2] - http://elixir.free-electrons.com/linux/v4.13-rc6/source/fs/sync.c#L183
Signed-off-by: Lukasz Majewski lukma@denx.de --- tools/env/fw_env.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index c9c79e0..ab7d85b 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1088,14 +1088,16 @@ static int flash_io (int mode)
rc = flash_write (fd_current, fd_target, dev_target);
- if (fsync (fd_current)) { + if (fsync(fd_current) && + !(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno)); }
if (HaveRedundEnv) { - if (fsync (fd_target)) { + if (fsync(fd_target) && + !(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno));
Hi Lukasz,
Am Sonntag, 27. August 2017, 13:46:22 CEST schrieb Lukasz Majewski:
According to fsync specification [1] some special files (e.g., a pipe, FIFO, or socket) don't support synchronization and return either EROFS or EINVAL.
On the linux side the sys_fsync -> do_fsync() checks if the requested file has f_op->fsync defined. If not it returns EINVAL [2].
This commit prevents writing error messages for files (devices), which do not support fsync().
[1] - http://man7.org/linux/man-pages/man2/fsync.2.html [2] - http://elixir.free-electrons.com/linux/v4.13-rc6/source/fs/sync.c#L183
Signed-off-by: Lukasz Majewski lukma@denx.de
tools/env/fw_env.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index c9c79e0..ab7d85b 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1088,14 +1088,16 @@ static int flash_io (int mode)
rc = flash_write (fd_current, fd_target, dev_target);
if (fsync (fd_current)) {
if (fsync(fd_current) &&
nitpick: current coding style seems to be a single whitespace before the opening bracket
!(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno));}
if (HaveRedundEnv) {
if (fsync (fd_target)) {
if (fsync(fd_target) &&
dito
!(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno));
Other than this nitpick:
Acked-by: Michael Heimpold mhei@heimpold.de
Regards, Michael
Hi Michael,
Hi Lukasz,
Am Sonntag, 27. August 2017, 13:46:22 CEST schrieb Lukasz Majewski:
According to fsync specification [1] some special files (e.g., a pipe, FIFO, or socket) don't support synchronization and return either EROFS or EINVAL.
On the linux side the sys_fsync -> do_fsync() checks if the requested file has f_op->fsync defined. If not it returns EINVAL [2].
This commit prevents writing error messages for files (devices), which do not support fsync().
[1] - http://man7.org/linux/man-pages/man2/fsync.2.html [2] - http://elixir.free-electrons.com/linux/v4.13-rc6/source/fs/sync.c#L183
Signed-off-by: Lukasz Majewski lukma@denx.de
tools/env/fw_env.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index c9c79e0..ab7d85b 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1088,14 +1088,16 @@ static int flash_io (int mode)
rc = flash_write (fd_current, fd_target, dev_target);
if (fsync (fd_current)) {
if (fsync(fd_current) &&nitpick: current coding style seems to be a single whitespace before the opening bracket
I've run the patch through the ./scripts/checkpatch.pl and it was complaining about this space, so I've corrected it.
Does the fw_env.c use another coding style (not from the Linux kernel)?
!(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno));}
if (HaveRedundEnv) {
if (fsync (fd_target)) {
if (fsync(fd_target) &&dito
!(errno == EINVAL || errno == EROFS)) { fprintf (stderr, "fsync failed on %s: %s\n", DEVNAME (dev_current), strerror (errno));Other than this nitpick:
Acked-by: Michael Heimpold mhei@heimpold.de
Thanks :-)
Regards, Michael
On Sun, Aug 27, 2017 at 09:20:19PM +0200, Łukasz Majewski wrote:
Hi Michael,
Hi Lukasz,
Am Sonntag, 27. August 2017, 13:46:22 CEST schrieb Lukasz Majewski:
According to fsync specification [1] some special files (e.g., a pipe, FIFO, or socket) don't support synchronization and return either EROFS or EINVAL.
On the linux side the sys_fsync -> do_fsync() checks if the requested file has f_op->fsync defined. If not it returns EINVAL [2].
This commit prevents writing error messages for files (devices), which do not support fsync().
[1] - http://man7.org/linux/man-pages/man2/fsync.2.html [2] - http://elixir.free-electrons.com/linux/v4.13-rc6/source/fs/sync.c#L183
Signed-off-by: Lukasz Majewski lukma@denx.de
tools/env/fw_env.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index c9c79e0..ab7d85b 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1088,14 +1088,16 @@ static int flash_io (int mode)
rc = flash_write (fd_current, fd_target, dev_target);
if (fsync (fd_current)) {
if (fsync(fd_current) &&nitpick: current coding style seems to be a single whitespace before the opening bracket
I've run the patch through the ./scripts/checkpatch.pl and it was complaining about this space, so I've corrected it.
Does the fw_env.c use another coding style (not from the Linux kernel)?
fw_env.c is another file that needs to be updated, stylistically to match the rest of U-Boot.
On Sun, Aug 27, 2017 at 01:46:22PM +0200, Lukasz Majewski wrote:
According to fsync specification [1] some special files (e.g., a pipe, FIFO, or socket) don't support synchronization and return either EROFS or EINVAL.
On the linux side the sys_fsync -> do_fsync() checks if the requested file has f_op->fsync defined. If not it returns EINVAL [2].
This commit prevents writing error messages for files (devices), which do not support fsync().
[1] - http://man7.org/linux/man-pages/man2/fsync.2.html [2] - http://elixir.free-electrons.com/linux/v4.13-rc6/source/fs/sync.c#L183
Signed-off-by: Lukasz Majewski lukma@denx.de Acked-by: Michael Heimpold mhei@heimpold.de
Applied to u-boot/master, thanks!
participants (4)
-
Lukasz Majewski -
Michael Heimpold -
Tom Rini -
Łukasz Majewski