Re: [PATCH]QNX6 filesystem (RO) driver

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sun, Feb 12, 2012 at 11:14:38PM +0100, Kai Bankett wrote:

re junk removal from qnx4 - sure, just make is a separate patch.

> >qnx6_iget() - what, they really don't allow named pipes/sockets/device
> >nodes on the filesystem?  If not, you can just use init_special_inode()
> >instead of that printk+iget_failed().
> At least so far I could not find any additional extras.
> I followed your advise and used init_special_inode() to get rid of
> that printk+stuff.

Umm...  How does qnx encode device numbers for block/character devices?

> Yes, I have the name length in the directory entry, but only for
> short filenames.
> For long filenames the length field in the directory entry is always 0xff.
> I just moved the length check up by one if-clause.
> Personally I don't think that moving it around further makes that
> much sense, as the length for short filenames is also checked before
> copying the filename.
> The way it currently is it's at least symmetrical between short und
> long filename handling code.
> 
> I hope I got all the points and fixes right?

A few more things:
	* duplicating di_block_ptr[] array seems to be pointless
	* qnx6_get_inode_loc(), qnx6_dir_lfile_block() and qnx6_block_map()
seem to have a lot of duplicated code, at the very least.  Looks like
a missing helper function...  Incidentally, I'd switched qnx6_get_devblock()
and qnx6_check_blockptr() to __fs32 argument, along with making QNX6_PTR_UNSET
(~(__fs32)0)
	* what is actually stored in ->de_size for long entries in case
of big-endian fs?  4 bytes of de_inode - fine, as as for short ones, but
then what?  If it's really 32bit big-endian 0xff, it will have the first byte
equal to 0, which would confuse the living hell out of your code.  Or is
it actually __u8 + 3 bytes of padding?  Then it would be compatible with
short dir entries, but conversions would be the wrong thing to do there...
	* I seriously suspect that you want to cook yourself a couple of
unhashed in-core struct inode for Longfile and Inode ones; then get_inode_loc(),
dir_lfile_block() and block_map() would become identical *and* you just might
be able to use page cache instead of all that messing with buffer_heads in
readdir et.al.  Just do new_inode() and fill di_block_ptr/levels/i_mode/i_size
and ->a_ops.  iput() both in ->put_super() and that's it...  For directories
in pagecache see how e.g. ext2 is doing it - or minixfs, for that matter.
	* mmu_private is never used.  Just lose it...