atari partitioning problem?

atari partitioning problem?

[Kenneth D. Merry]

[ Al Viro CCed, since from the changelog for 2.4.11, it looks like he may
have submitted the patch in question. ]

I was looking through the atari partitioning code (fs/partitions/atari.c)
in 2.4.18 (from what I can see in 2.4.19-pre8, it hasn't changed) and I
noticed something (starting at about line 144) that may be a problem:

			for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, p
i++) {
				/* accept only GEM,BGM,RAW,LNX,SWP partitions */
				if (!((pi->flg & 1) && OK_id(pi->id)))
					continue;
				part_fmt = 2;
				add_gd_partition (hd, minor,
						  be32_to_cpu(pi->st),
						  be32_to_cpu(pi->siz));
			}
			printk(" >");

This code changed in 2.4.11, and was previously:

      for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, pi++)
      { 
        /* accept only GEM,BGM,RAW,LNX,SWP partitions */
        if (pi->flg & 1 &&
            (memcmp (pi->id, "GEM", 3) == 0 ||
             memcmp (pi->id, "BGM", 3) == 0 ||
             memcmp (pi->id, "LNX", 3) == 0 ||
             memcmp (pi->id, "SWP", 3) == 0 ||
             memcmp (pi->id, "RAW", 3) == 0) )
        { 
          part_fmt = 2;
          add_gd_partition (hd, minor, be32_to_cpu(pi->st),
                            be32_to_cpu(pi->siz));
        }
      }
      printk(" >");

Note that the test was reversed, but not fully.  You want only active
partitions of the specified types to be added, but with the code as it is
now, only partitions that are active and *not* of the correct type will get
added.

I think that this patch would probably fix the problem, although I don't
have any hardware to test it.


==== //depot/linux-2.4.18/fs/partitions/atari.c#1 - /usr/home/ken/perforce/linux-2.4.18/fs/partitions/atari.c ====
--- /tmp/tmp.20618.0	Mon May 13 17:01:23 2002
+++ /usr/home/ken/perforce/linux-2.4.18/fs/partitions/atari.c	Mon May 13 16:34:27 2002
@@ -142,7 +142,7 @@
 			printk(" ICD<");
 			for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, pi++) {
 				/* accept only GEM,BGM,RAW,LNX,SWP partitions */
-				if (!((pi->flg & 1) && OK_id(pi->id)))
+				if (!((pi->flg & 1) || !OK_id(pi->id)))
 					continue;
 				part_fmt = 2;
 				add_gd_partition (hd, minor,

Ken
-- 
Kenneth Merry
ken@kdm.org

Re: atari partitioning problem?

[Kai Germaschewski]

On Mon, 13 May 2002, Kenneth D. Merry wrote:

> [ Al Viro CCed, since from the changelog for 2.4.11, it looks like he may
> have submitted the patch in question. ]
> 
> I was looking through the atari partitioning code (fs/partitions/atari.c)
> in 2.4.18 (from what I can see in 2.4.19-pre8, it hasn't changed) and I
> noticed something (starting at about line 144) that may be a problem:
> 
> 			for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, p
> i++) {
> 				/* accept only GEM,BGM,RAW,LNX,SWP partitions */
> 				if (!((pi->flg & 1) && OK_id(pi->id)))
> 					continue;
> 				part_fmt = 2;
> 				add_gd_partition (hd, minor,
> 						  be32_to_cpu(pi->st),
> 						  be32_to_cpu(pi->siz));
> 			}
> 			printk(" >");
> 
> This code changed in 2.4.11, and was previously:
> 
>       for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, pi++)
>       { 
>         /* accept only GEM,BGM,RAW,LNX,SWP partitions */
>         if (pi->flg & 1 &&
>             (memcmp (pi->id, "GEM", 3) == 0 ||
>              memcmp (pi->id, "BGM", 3) == 0 ||
>              memcmp (pi->id, "LNX", 3) == 0 ||
>              memcmp (pi->id, "SWP", 3) == 0 ||
>              memcmp (pi->id, "RAW", 3) == 0) )
>         { 
>           part_fmt = 2;
>           add_gd_partition (hd, minor, be32_to_cpu(pi->st),
>                             be32_to_cpu(pi->siz));
>         }
>       }
>       printk(" >");
> 
> Note that the test was reversed, but not fully.  You want only active
> partitions of the specified types to be added, but with the code as it is
> now, only partitions that are active and *not* of the correct type will get
> added.

No, the change is correct.

Note that the ! in the first snippet is in front of the whole expression, 
and it's pretty obvious that the reversed test for (a && b) is !(a && b).

The latter is equivalent to (!a || !b), which seems to be what you're 
thinking, but you didn't get it right and put !(a || !b) which is wrong.

--Kai

Re: atari partitioning problem?

[Kenneth D. Merry]

On Mon, May 13, 2002 at 19:25:17 -0500, Kai Germaschewski wrote:
> On Mon, 13 May 2002, Kenneth D. Merry wrote:
> 
> > [ Al Viro CCed, since from the changelog for 2.4.11, it looks like he may
> > have submitted the patch in question. ]
> > 
> > I was looking through the atari partitioning code (fs/partitions/atari.c)
> > in 2.4.18 (from what I can see in 2.4.19-pre8, it hasn't changed) and I
> > noticed something (starting at about line 144) that may be a problem:
> > 
> > 			for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, p
> > i++) {
> > 				/* accept only GEM,BGM,RAW,LNX,SWP partitions */
> > 				if (!((pi->flg & 1) && OK_id(pi->id)))
> > 					continue;
> > 				part_fmt = 2;
> > 				add_gd_partition (hd, minor,
> > 						  be32_to_cpu(pi->st),
> > 						  be32_to_cpu(pi->siz));
> > 			}
> > 			printk(" >");
> > 
> > This code changed in 2.4.11, and was previously:
> > 
> >       for (; pi < &rs->icdpart[8] && minor < m_lim; minor++, pi++)
> >       { 
> >         /* accept only GEM,BGM,RAW,LNX,SWP partitions */
> >         if (pi->flg & 1 &&
> >             (memcmp (pi->id, "GEM", 3) == 0 ||
> >              memcmp (pi->id, "BGM", 3) == 0 ||
> >              memcmp (pi->id, "LNX", 3) == 0 ||
> >              memcmp (pi->id, "SWP", 3) == 0 ||
> >              memcmp (pi->id, "RAW", 3) == 0) )
> >         { 
> >           part_fmt = 2;
> >           add_gd_partition (hd, minor, be32_to_cpu(pi->st),
> >                             be32_to_cpu(pi->siz));
> >         }
> >       }
> >       printk(" >");
> > 
> > Note that the test was reversed, but not fully.  You want only active
> > partitions of the specified types to be added, but with the code as it is
> > now, only partitions that are active and *not* of the correct type will get
> > added.
> 
> No, the change is correct.
> 
> Note that the ! in the first snippet is in front of the whole expression, 
> and it's pretty obvious that the reversed test for (a && b) is !(a && b).
> 
> The latter is equivalent to (!a || !b), which seems to be what you're 
> thinking, but you didn't get it right and put !(a || !b) which is wrong.

Oops!  You're right, thanks for pointing that out. :)

Ken
-- 
Kenneth Merry
ken@kdm.org