All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Lubomir Rintel <lkundrak@v3.sk>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] V7: Add support for non-PDP11 v7 filesystems
Date: Mon, 26 Jul 2010 17:29:16 -0700	[thread overview]
Message-ID: <20100726172916.50ab7356.akpm@linux-foundation.org> (raw)
In-Reply-To: <1280189955.26200.5.camel@localhost.localdomain>

On Tue, 27 Jul 2010 02:19:15 +0200 Lubomir Rintel <lkundrak@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@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@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;
_


  reply	other threads:[~2010-07-27  0:29 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-19 17:16 A few V7 fs improvements Lubomir Rintel
2010-07-19 14:53 ` Christoph Hellwig
2010-07-19 16:10 ` Bill Davidsen
2010-07-19 17:16 ` [PATCH 1/3] [fs/sysv] Add v7 alias Lubomir Rintel
2010-07-19 14:53   ` Christoph Hellwig
2010-07-19 17:16   ` [PATCH 2/3] [fs/sysv] V7: Adjust sanity checks for some volumes Lubomir Rintel
2010-07-19 14:54     ` Christoph Hellwig
2010-07-20 10:31       ` Lubomir Rintel
2010-07-22  1:11         ` [PATCH] " Lubomir Rintel
2010-07-19 17:16     ` [PATCH 3/3] [fs/sysv] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
2010-07-19 14:58       ` Christoph Hellwig
2010-07-20 10:41         ` Lubomir Rintel
2010-07-22  1:17           ` Lubomir Rintel
2010-07-22  1:18             ` [PATCH] " Lubomir Rintel
2010-07-24 13:51         ` [PATCH 3/3] [fs/sysv] " Al Viro
2010-07-25 22:59           ` [PATCH] " Lubomir Rintel
2010-07-26  0:26             ` Randy Dunlap
2010-07-26 23:52             ` Andrew Morton
2010-07-27  0:19               ` Lubomir Rintel
2010-07-27  0:29                 ` Andrew Morton [this message]
2010-07-25  5:23 ` A few V7 fs improvements Artem Bityutskiy
  -- strict thread matches above, loose matches on Subject: below --
2010-07-25 22:50 [PATCH] V7: Add support for non-PDP11 v7 filesystems Lubomir Rintel
2010-07-25 22:54 ` Lubomir Rintel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100726172916.50ab7356.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkundrak@v3.sk \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.