wrong count argument to erase()

wrong count argument to erase()

[Giorgio]

[-- Attachment #1.1.1: Type: text/plain, Size: 1110 bytes --]

Hi,

I'm using barebox on an embedded system with an imx6 cpu, a nor and
a nand flash.

I recently updated the barebox to v2016.04.0 and noticed that the
command 'saveenv' was surprisingly quick and actually didn't work.

After a bit of debugging I could track the problem down to a call to the
function erase() within 'common/environment.c':

int envfs_save(const char *filename, const char *dirname, unsigned flags)
{
...
	ret = erase(envfd, ~0, 0);

	/* ENOSYS and EOPNOTSUPP aren't errors here, many devices don't need it */
	if (ret && errno != ENOSYS && errno != EOPNOTSUPP) {
		printf("could not erase %s: %s\n", filename, errno_str());
		goto out;
	}
...

The function prototype is:

int erase(int fd, loff_t count, loff_t offset)

in particular the second argument, count, is a signed long long.

Now when calling erase() with a count of ~0 the function will cast it
to -1 instead of to a 'very big' number and actually never erase
anything.

I think in these cases it is better to use cpp macros like LLONG_MAX
or change the type of count to a size_t.

giorgio


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 246 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: wrong count argument to erase()

[Sascha Hauer]

Hi Giorgio,

On Sat, Apr 16, 2016 at 09:15:04PM +0200, Giorgio wrote:
> Hi,
> 
> I'm using barebox on an embedded system with an imx6 cpu, a nor and
> a nand flash.
> 
> I recently updated the barebox to v2016.04.0 and noticed that the
> command 'saveenv' was surprisingly quick and actually didn't work.
> 
> After a bit of debugging I could track the problem down to a call to the
> function erase() within 'common/environment.c':
> 
> int envfs_save(const char *filename, const char *dirname, unsigned flags)
> {
> ...
> 	ret = erase(envfd, ~0, 0);
> 
> 	/* ENOSYS and EOPNOTSUPP aren't errors here, many devices don't need it */
> 	if (ret && errno != ENOSYS && errno != EOPNOTSUPP) {
> 		printf("could not erase %s: %s\n", filename, errno_str());
> 		goto out;
> 	}
> ...
> 
> The function prototype is:
> 
> int erase(int fd, loff_t count, loff_t offset)
> 
> in particular the second argument, count, is a signed long long.
> 
> Now when calling erase() with a count of ~0 the function will cast it
> to -1 instead of to a 'very big' number and actually never erase
> anything.
> 
> I think in these cases it is better to use cpp macros like LLONG_MAX
> or change the type of count to a size_t.

Thanks for reporting this. I stumbled over the same issue a moment ago.
I just sent a patch fixing this to the list.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox