Re: fsck.minix broken

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 Thanks Andries for your report and debugging,

On Mon, Oct 05, 2009 at 11:39:21PM +0200, Andries E. Brouwer wrote:
> static inline int bit(char * addr,unsigned int nr)
> {
>   return (addr[nr >> 3] & (1<<(nr & 7))) != 0;
> }

 yes, I have added != 0 to [zone,inode]_in_use() macros to fix the
 problem. It seems it works now. (Fortunately, it's easy to reproduce
 the problem.)

> Nowadays, I see in 2.16.1:
>   #define zone_in_use(x) (isset(zone_map,(x)-FIRSTZONE+1))
> which uses the undocumented isset() function - bad.

 Well, I have no clue why these bitmap macros are undocumented, but
 you can found them everywhere (cygwin, glibc, uClibc, BSD, ...). It
 seems that theses macros are missing in klibc only. So I have added a
 fallback definition to bitops.h.

> The easy solution is to revert to earlier code in mkfs.minix

 I don't thinks so. It's much better to maintain a small macro (or use
 the one from libc) than the old assembler implementation.

    Karel

>From e98754d6bbd277902f4e5e8190b29ece5d4376ec Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@xxxxxxxxxx>
Date: Thu, 15 Oct 2009 14:14:32 +0200
Subject: [PATCH 1/2] fsck.minix: fix broken zone checking

This bug has been introduced by commit
95356e8b744439336925eeb36f01399f1ee8a5e9.

The fsck.minix code assumes that isset() macro returns boolean,
unfortunately the generic implementation from libc returns integer.

This patch also add a fallback for the bitmap macros to include/bitops.h.

Reported-by: "Andries E. Brouwer" <Andries.Brouwer@xxxxxx>
Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
---
 disk-utils/fsck.minix.c |    6 +++---
 disk-utils/mkfs.minix.c |    5 ++---
 include/bitops.h        |   16 ++++++++++++++++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/disk-utils/fsck.minix.c b/disk-utils/fsck.minix.c
index c9bd653..31b5ef6 100644
--- a/disk-utils/fsck.minix.c
+++ b/disk-utils/fsck.minix.c
@@ -100,12 +100,12 @@
 #include <termios.h>
 #include <mntent.h>
 #include <sys/stat.h>
-#include <sys/param.h>
 #include <signal.h>
 
 #include "minix.h"
 #include "nls.h"
 #include "pathnames.h"
+#include "bitops.h"
 
 #define ROOT_INO 1
 
@@ -166,8 +166,8 @@ static unsigned char * zone_count = NULL;
 static void recursive_check(unsigned int ino);
 static void recursive_check2(unsigned int ino);
 
-#define inode_in_use(x) (isset(inode_map,(x)))
-#define zone_in_use(x) (isset(zone_map,(x)-FIRSTZONE+1))
+#define inode_in_use(x) (isset(inode_map,(x)) != 0)
+#define zone_in_use(x) (isset(zone_map,(x)-FIRSTZONE+1) != 0)
 
 #define mark_inode(x) (setbit(inode_map,(x)),changed=1)
 #define unmark_inode(x) (clrbit(inode_map,(x)),changed=1)
diff --git a/disk-utils/mkfs.minix.c b/disk-utils/mkfs.minix.c
index ebef518..391f09b 100644
--- a/disk-utils/mkfs.minix.c
+++ b/disk-utils/mkfs.minix.c
@@ -68,7 +68,6 @@
 #include <stdlib.h>
 #include <termios.h>
 #include <sys/stat.h>
-#include <sys/param.h>
 #include <mntent.h>
 #include <getopt.h>
 
@@ -76,6 +75,7 @@
 #include "minix.h"
 #include "nls.h"
 #include "pathnames.h"
+#include "bitops.h"
 
 #define MINIX_ROOT_INO 1
 #define MINIX_BAD_INO 2
@@ -131,8 +131,7 @@ static unsigned short good_blocks_table[MAX_GOOD_BLOCKS];
 static int used_good_blocks = 0;
 static unsigned long req_nr_inodes = 0;
 
-#define inode_in_use(x) (isset(inode_map,(x)))
-#define zone_in_use(x) (isset(zone_map,(x)-FIRSTZONE+1))
+#define zone_in_use(x) (isset(zone_map,(x)-FIRSTZONE+1) != 0)
 
 #define mark_inode(x) (setbit(inode_map,(x)))
 #define unmark_inode(x) (clrbit(inode_map,(x)))
diff --git a/include/bitops.h b/include/bitops.h
index e6eaff1..e283b83 100644
--- a/include/bitops.h
+++ b/include/bitops.h
@@ -4,6 +4,22 @@
 #include <stdint.h>
 #include <endian.h>
 
+/*
+ * Bit map related macros. Usually provided by libc.
+ */
+#include <sys/param.h>
+
+#ifndef NBBY
+# define NBBY            CHAR_BIT
+#endif
+
+#ifndef setbit
+# define setbit(a,i)	((a)[(i)/NBBY] |= 1<<((i)%NBBY))
+# define clrbit(a,i)	((a)[(i)/NBBY] &= ~(1<<((i)%NBBY)))
+# define isset(a,i)	((a)[(i)/NBBY] & (1<<((i)%NBBY)))
+# define isclr(a,i)	(((a)[(i)/NBBY] & (1<<((i)%NBBY))) == 0)
+#endif
+
 #if !defined __BYTE_ORDER || !(__BYTE_ORDER == __LITTLE_ENDIAN) && !(__BYTE_ORDER == __BIG_ENDIAN)
 #error missing __BYTE_ORDER
 #endif
-- 
1.6.2.5

--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux