Barebox Enviroment FileSystem

Barebox Enviroment FileSystem

[andreas.willig]

Hi there

I'm just about to implement direct access to BareBox Env FileSystem from
Windows Embedded Compact 7 launched through BB and it seems to me I met a
bug in the environment code.

File: common/environment.c
Line: 89 (latest revision by 8th of May)
Function: file_save_action

The creation of the File's INODE is done strange:

  87         inode = (struct envfs_inode*)data->writep;
  88         inode->magic = ENVFS_32(ENVFS_INODE_MAGIC);
  89         inode->headerlen = ENVFS_32(PAD4(namelen + sizeof(struct
envfs_inode_end)));

Now the bug / my question is:
Why is the HeaderLen of the Inode created by padded filename + sizeof
inode_end instead of padded filename + sizeof(inode) did i get something
wrong or is this a bug?

envfs_load will not meet this problem since headerlen is not used during
expansion of FS to ram.

An additional question i got btw: Is it intentionally desired that empty
directories are dropped during creation of bin file? The inode_end seems to
support an empty directory entry, but code does not create such entries.

Thanks in advance

Andreas


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

Re: Barebox Enviroment FileSystem

[Sascha Hauer]

On Wed, May 08, 2013 at 10:27:39AM +0200, andreas.willig@rafi.de wrote:
> 
> Hi there
> 
> I'm just about to implement direct access to BareBox Env FileSystem from
> Windows Embedded Compact 7 launched through BB and it seems to me I met a
> bug in the environment code.
> 
> File: common/environment.c
> Line: 89 (latest revision by 8th of May)
> Function: file_save_action
> 
> The creation of the File's INODE is done strange:
> 
>   87         inode = (struct envfs_inode*)data->writep;
>   88         inode->magic = ENVFS_32(ENVFS_INODE_MAGIC);
>   89         inode->headerlen = ENVFS_32(PAD4(namelen + sizeof(struct
> envfs_inode_end)));
> 
> Now the bug / my question is:
> Why is the HeaderLen of the Inode created by padded filename + sizeof
> inode_end instead of padded filename + sizeof(inode) did i get something
> wrong or is this a bug?

No, it's not a bug. On storage we have:

|-- struct inode --||-- name --||-- struct inode_end --| <next inode>

headerlen is the length of name and struct inode_end. To understand the
layout it helps to look at the history. Originally we had:

|-- struct inode --||-- name --| <next inode>

The field 'headerlen' was named 'namelen' back then. Then
Jean-Christophe wanted to add a new data field (the file mode). The
obvious way of adding it to struct inode was not chosen because it would
have changed the storage data format. Instead a struct inode_end was
added which could be read by both the old and the new code.

I think more correctly the line you reference should read:

inode->headerlen = ENVFS_32(PAD4(namelen) + sizeof(struct envfs_inode_end));

But the result is the same.

> 
> envfs_load will not meet this problem since headerlen is not used during
> expansion of FS to ram.
> 
> An additional question i got btw: Is it intentionally desired that empty
> directories are dropped during creation of bin file? The inode_end seems to
> support an empty directory entry, but code does not create such entries.

No, this was not intentional. You're probably the first one who ever
spotted this.
This might be worth fixing, but appearently this doesn't seem to be very
important ;)

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