From: Lubomir Rintel on
On Mon, 2010-07-26 at 00:50 +0200, Lubomir Rintel wrote:
> A mount-time option was added that makes it possible to override the
> endianness and an attempt is made to autodetect it (which seems easy,
> given the disk addresses are 3-byte.
>
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

This should have been a reply to
<20100724135128.GA11645(a)ZenIV.linux.org.uk>
Moreover it has a bad commit message; By bad, please ignore this one.

sorry
--
Flash is the Web2.0 version of blink and animated gifs.
-- Stephen Smoogen

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Randy Dunlap on
On Mon, 26 Jul 2010 00:59:16 +0200 Lubomir Rintel wrote:

> This adds byte order autodetection (of PDP-11 and LE filesystems).
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
>
> Cc: Christoph Hellwig <hch(a)lst.de>
> Cc: Al Viro <viro(a)ZenIV.linux.org.uk>
> Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
> ---
> fs/sysv/super.c | 74 +++++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 50 insertions(+), 24 deletions(-)
>
> diff --git a/fs/sysv/super.c b/fs/sysv/super.c
> index ac7b008..0433f23 100644
> --- a/fs/sysv/super.c
> +++ b/fs/sysv/super.c
> @@ -435,12 +436,46 @@ Ebadsize:
> goto failed;
> }
>
> -static int v7_fill_super(struct super_block *sb, void *data, int silent)
> +static int v7_sanity_check(struct super_block *sb, struct buffer_head *bh)
> {
> - struct sysv_sb_info *sbi;
> - struct buffer_head *bh, *bh2 = NULL;
> struct v7_super_block *v7sb;
> struct sysv_inode *v7i;
> + struct buffer_head *bh2;
> + struct sysv_sb_info *sbi;
> +
> + sbi = sb->s_fs_info;
> +
> + /* plausibility check on superblock */
> + v7sb = (struct v7_super_block *) bh->b_data;
> + if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
> + fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
> + fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
> + return 0;
> +
> + /* plausibility check on root inode: it is a directory,
> + with a nonzero size that is a multiple of 16 */
> + if ((bh2 = sb_bread(sb, 2)) == NULL) {
> + return 0;
> + }

Don't need the braces.

> +
> + v7i = (struct sysv_inode *)(bh2->b_data + 64);
> + if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
> + (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> + (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
> + (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> + sizeof (struct sysv_dir_entry))) {

sizeof(struct sysv_dir_entry))) {

> + brelse(bh2);
> + return 0;
> + }
> +
> + brelse(bh2);
> + return 1;
> +}
> +
> +static int v7_fill_super(struct super_block *sb, void *data, int silent)
> +{
> + struct sysv_sb_info *sbi;
> + struct buffer_head *bh;
>
> if (440 != sizeof (struct v7_super_block))
> panic("V7 FS: bad super-block size");
> @@ -454,7 +489,6 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
> sbi->s_sb = sb;
> sbi->s_block_base = 0;
> sbi->s_type = FSTYPE_V7;
> - sbi->s_bytesex = BYTESEX_PDP;
> sb->s_fs_info = sbi;
>
> sb_set_blocksize(sb, 512);
> @@ -466,34 +500,26 @@ static int v7_fill_super(struct super_block *sb, void *data, int silent)
> goto failed;
> }
>
> - /* plausibility check on superblock */
> - v7sb = (struct v7_super_block *) bh->b_data;
> - if (fs16_to_cpu(sbi, v7sb->s_nfree) > V7_NICFREE ||
> - fs16_to_cpu(sbi, v7sb->s_ninode) > V7_NICINOD ||
> - fs32_to_cpu(sbi, v7sb->s_fsize) > V7_MAXSIZE)
> - goto failed;
> + /* Try PDP-11 UNIX */
> + sbi->s_bytesex = BYTESEX_PDP;
> + if (v7_sanity_check (sb, bh))
> + goto detected;
>
> - /* plausibility check on root inode: it is a directory,
> - with a nonzero size that is a multiple of 16 */
> - if ((bh2 = sb_bread(sb, 2)) == NULL)
> - goto failed;
> - v7i = (struct sysv_inode *)(bh2->b_data + 64);
> - if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
> - (fs32_to_cpu(sbi, v7i->i_size) == 0) ||
> - (fs32_to_cpu(sbi, v7i->i_size) & 017) ||
> - (fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
> - sizeof (struct sysv_dir_entry)))
> - goto failed;
> - brelse(bh2);
> - bh2 = NULL;
> + /* Try PC/IX, v7/x86 */
> + sbi->s_bytesex = BYTESEX_LE;
> + if (v7_sanity_check (sb, bh))
> + goto detected;
>
> + goto failed;
> +
> +detected:
> sbi->s_bh1 = bh;
> sbi->s_bh2 = bh;
> if (complete_read_super(sb, silent, 1))
> return 0;
>
> failed:
> - brelse(bh2);
> + printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);

V7 fs (or V7 filesystem)

> brelse(bh);
> kfree(sbi);
> return -EINVAL;
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Mon, 26 Jul 2010 00:59:16 +0200
Lubomir Rintel <lkundrak(a)v3.sk> wrote:

> This adds byte order autodetection (of PDP-11 and LE filesystems).
> No attempt is made to detect big-endian filesystems -- were there any?
> Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.

Which kernel is this against? My v7_fill_super() is significantly
different from yours.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Lubomir Rintel on
On Mon, 2010-07-26 at 16:52 -0700, Andrew Morton wrote:
> On Mon, 26 Jul 2010 00:59:16 +0200
> Lubomir Rintel <lkundrak(a)v3.sk> wrote:
>
> > This adds byte order autodetection (of PDP-11 and LE filesystems).
> > No attempt is made to detect big-endian filesystems -- were there any?
> > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
>
> Which kernel is this against? My v7_fill_super() is significantly
> different from yours.

This depends on [PATCH] V7: Adjust sanity checks for some volumes
<1279761108-302-1-git-send-email-lkundrak(a)v3.sk>.

It might be that I've messed up something in the communication so that
the dependency was not immediately clear. To be honest, I'm not very
familiar with the way reviews and adjustements to patches are tracked.
What have I done wrong, here? How am I supposed to follow up with a
change to a single patch of a multi-patch (be it just two in this case)
set?

--
Flash is the Web2.0 version of blink and animated gifs.
-- Stephen Smoogen

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Tue, 27 Jul 2010 02:19:15 +0200 Lubomir Rintel <lkundrak(a)v3.sk> wrote:

> On Mon, 2010-07-26 at 16:52 -0700, Andrew Morton wrote:
> > On Mon, 26 Jul 2010 00:59:16 +0200
> > Lubomir Rintel <lkundrak(a)v3.sk> wrote:
> >
> > > This adds byte order autodetection (of PDP-11 and LE filesystems).
> > > No attempt is made to detect big-endian filesystems -- were there any?
> > > Tested with PDP-11 v7 filesystems and PC-IX maintenance floppy.
> >
> > Which kernel is this against? My v7_fill_super() is significantly
> > different from yours.
>
> This depends on [PATCH] V7: Adjust sanity checks for some volumes
> <1279761108-302-1-git-send-email-lkundrak(a)v3.sk>.
>
> It might be that I've messed up something in the communication so that
> the dependency was not immediately clear. To be honest, I'm not very
> familiar with the way reviews and adjustements to patches are tracked.
> What have I done wrong, here? How am I supposed to follow up with a
> change to a single patch of a multi-patch (be it just two in this case)
> set?

ah, OK, I managed to confuse myself with
fs-sysv-v7-adjust-sanity-checks-for-some-volumes-checkpatch-fixes.patch.

This new patch triggers many warnings also:

ERROR: do not use assignment in if condition
#52: FILE: fs/sysv/super.c:456:
+ if ((bh2 = sb_bread(sb, 2)) == NULL) {

WARNING: braces {} are not necessary for single statement blocks
#52: FILE: fs/sysv/super.c:456:
+ if ((bh2 = sb_bread(sb, 2)) == NULL) {
+ return 0;
+ }

ERROR: code indent should use tabs where possible
#61: FILE: fs/sysv/super.c:465:
+ sizeof (struct sysv_dir_entry))) {$

WARNING: please, no space for starting a line,
excluding comments
#61: FILE: fs/sysv/super.c:465:
+ sizeof (struct sysv_dir_entry))) {$

WARNING: space prohibited between function name and open parenthesis '('
#61: FILE: fs/sysv/super.c:465:
+ sizeof (struct sysv_dir_entry))) {

WARNING: space prohibited between function name and open parenthesis '('
#97: FILE: fs/sysv/super.c:504:
+ if (v7_sanity_check (sb, bh))

WARNING: space prohibited between function name and open parenthesis '('
#115: FILE: fs/sysv/super.c:509:
+ if (v7_sanity_check (sb, bh))

WARNING: printk() should include KERN_ facility level
#128: FILE: fs/sysv/super.c:521:
+ printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);

total: 2 errors, 6 warnings, 109 lines checked

../patches/v7-add-support-for-non-pdp11-v7-filesystems.patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.




--- a/fs/sysv/super.c~fs-sysv-superc-add-support-for-non-pdp11-v7-filesystems-checkpatch-fixes
+++ a/fs/sysv/super.c
@@ -453,16 +453,16 @@ static int v7_sanity_check(struct super_

/* plausibility check on root inode: it is a directory,
with a nonzero size that is a multiple of 16 */
- if ((bh2 = sb_bread(sb, 2)) == NULL) {
+ bh2 = sb_bread(sb, 2);
+ if (bh2 == NULL)
return 0;
- }

v7i = (struct sysv_inode *)(bh2->b_data + 64);
if ((fs16_to_cpu(sbi, v7i->i_mode) & ~0777) != S_IFDIR ||
(fs32_to_cpu(sbi, v7i->i_size) == 0) ||
(fs32_to_cpu(sbi, v7i->i_size) & 017) ||
(fs32_to_cpu(sbi, v7i->i_size) > V7_NFILES *
- sizeof (struct sysv_dir_entry))) {
+ sizeof(struct sysv_dir_entry))) {
brelse(bh2);
return 0;
}
@@ -501,12 +501,12 @@ static int v7_fill_super(struct super_bl

/* Try PDP-11 UNIX */
sbi->s_bytesex = BYTESEX_PDP;
- if (v7_sanity_check (sb, bh))
+ if (v7_sanity_check(sb, bh))
goto detected;

/* Try PC/IX, v7/x86 */
sbi->s_bytesex = BYTESEX_LE;
- if (v7_sanity_check (sb, bh))
+ if (v7_sanity_check(sb, bh))
goto detected;

goto failed;
@@ -518,7 +518,8 @@ detected:
return 0;

failed:
- printk("VFS: could not find a valid V7 on %s.\n", sb->s_id);
+ printk(KERN_ERR "VFS: could not find a valid V7 on %s.\n",
+ sb->s_id);
brelse(bh);
kfree(sbi);
return -EINVAL;
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/