[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[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) &&
+		    !(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));
-- 
2.1.4

[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[Michael Heimpold]

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

[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[Łukasz Majewski]

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
>
>


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[Tom Rini]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1896 bytes --]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170827/113d581b/attachment.sig>

[U-Boot] fix: fw_env: Prevent writing error message on special files, which don't support fsync

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170903/d47dc28d/attachment.sig>