Re: [OpenBIOS] Patch for auto partition selection

[Programmingkid]

On Dec 4, 2012, at 1:48 PM, Blue Swirl wrote:
> On Tue, Dec 4, 2012 at 1:25 AM, Programmingkid
>  wrote:
> > On Dec 3, 2012, at 3:13 PM, Mark Cave-Ayland wrote:
> >
> > On 30/11/12 20:14, Programmingkid wrote:
> >
> > What do you mean when you say something is whitespace damaged?
> >
> > It's a general term for formatting changes that don't affect the patch, e.g.
> >
> > for (i = 0; i <= 10; i++) {
> >
> > to
> >
> > for (i = 0; i <= 10; i++)
> > {
>
> Please take a look at this example for non-wanted style changes. Don't
> just ignore what others are telling you.
> > It means that the patch includes non-functional changes which is generally
> > frowned upon.
> >
> > What section do I not need? Could you give me the first and last line of
> > this section? I need the actual text since the line number will probably not
> > help if our files are different.
> >
> > See below.
> >
> > } else {
> >
> > /* Another partition was explicitly requested */
> >
> > }
> >
> > // if no partition and no file was selected - example: "dir cd:,"
> >
> > else if (parnum == -1)
> >
> > {
> >  // search for the first partition of type Apple_HFS or Apple_HFSX
> >
> >  for(parnum = 1; parnum<= __be32_to_cpu(par.pmMapBlkCnt); parnum++)
> >
> >  {
> >
> >     SEEK( bs * parnum );
> >
> >     READ(&par, sizeof(par) );
> >
> >     DPRINTF("found partition type: %s with status %x\n",
> >
> > par.pmPartType, __be32_to_cpu(par.pmPartStatus));
> >     if(strcmp(par.pmPartType, "Apple_HFS") == 0 ||
> >
> > strcmp(par.pmPartType, "Apple_HFSX") == 0)
> >     {
> >
> >           offs = (long long)__be32_to_cpu(par.pmPyPartStart) * bs;
> >
> >           size = (long long)__be32_to_cpu(par.pmPartBlkCnt) * bs;
> >
> >           goto found;
> >
> >           break;
> >
> >     }
> >
> >  }
> >
> > }
> >
> > // Another partition was explicitly requested
> >
> > else
> >
> > {
> >
> > ATB,
> >
> > Mark.
> >
> > Ok this patch removes the unneeded code and remotes the whitespace damage.
> >
> > signed-off-by: John Arbuckle
> > Index: trunk/openbios-devel/packages/mac-parts.c
> >
> > --- trunk/openbios-devel/packages/mac-parts.c (revision 1075)
> > +++ trunk/openbios-devel/packages/mac-parts.c (working copy)
> > @@ -164,16 +164,12 @@
> >  for (parnum = 1; parnum <= __be32_to_cpu(par.pmMapBlkCnt); parnum++) {
> >  SEEK( bs * parnum );
> >  READ( &par, sizeof(par) );
> > if( __be16_to_cpu(par.pmSig) != DESC_PART_SIGNATURE ||
> >                        !__be32_to_cpu(par.pmPartBlkCnt) )
> > break;
> >
> > DPRINTF("found partition type: %s with status %x\n", par.pmPartType,
> > __be32_to_cpu(par.pmPartStatus));
> > /* If we have a valid, allocated and readable partition... */
> > if( (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsValid) &&
> > (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsAllocated) &&
> > (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsReadable) ) {
> > // If an Apple_HFS or Apple_HFSX partition was found
> > if(strcmp(par.pmPartType, "Apple_HFS") == 0 || strcmp(par.pmPartType,
> >
> > "Apple_HFSX") == 0)
> > {
>
> Here's one case where you have changed the brace style.
> > offs = (long long)__be32_to_cpu(par.pmPyPartStart) * bs;
> >  size = (long long)__be32_to_cpu(par.pmPartBlkCnt) * bs;
> >
> > @@ -181,9 +177,11 @@
> >  if (want_bootcode && (__be32_to_cpu(par.pmPartStatus) &
> > kPartitionAUXIsBootValid)) {
> >  offs += (long long)__be32_to_cpu(par.pmLgBootStart) * bs;
> >  size = (long long)__be32_to_cpu(par.pmBootSize);
> >
> > goto found;
> > } else {
> > }
> > else
> > {
>
> Another, don't change '} else {'.
> > /* Otherwise we were passed a filename and path. So let's
> >    choose the first partition with a valid filesystem */
> >  DPUSH( offs );
> > @@ -196,9 +194,11 @@
> >  }
> >  }
> >  }
> > } else {
> > /* Another partition was explicitly requested */
> > }
> > // Another partition was explicitly requested
>
> And here you changed the comment style to C99. Don't do that.
> > else
> > {
>
> Another unwanted change.
>
> Please take a more care to follow the style, consistency is important
> in software projects. Ignoring review comments will lead to ignored
> patches.
> > SEEK( bs * parnum );
> >  READ( &par, sizeof(par) );

Here is an updated patch with the above issues fixed.
Signed-off-by : John Arbuckle

Index: trunk/openbios-devel/packages/mac-parts.c
===================================================================
--- trunk/openbios-devel/packages/mac-parts.c	(revision 1075)
+++ trunk/openbios-devel/packages/mac-parts.c	(working copy)
@@ -164,16 +164,10 @@
    	for (parnum = 1; parnum <= __be32_to_cpu(par.pmMapBlkCnt); parnum++) {
    		SEEK( bs * parnum );
    		READ( &par, sizeof(par) );
-			if( __be16_to_cpu(par.pmSig) != DESC_PART_SIGNATURE ||
-                            !__be32_to_cpu(par.pmPartBlkCnt) )
-				break;
-
    		DPRINTF("found partition type: %s with status %x\n", par.pmPartType, __be32_to_cpu(par.pmPartStatus));

-			/* If we have a valid, allocated and readable partition... */
-			if( (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsValid) &&
-			(__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsAllocated) &&
-			(__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsReadable) ) {
+			/* If an Apple_HFS or Apple_HFSX partition was found */
+			if(strcmp(par.pmPartType, "Apple_HFS") == 0 || strcmp(par.pmPartType, "Apple_HFSX") == 0) {
    			offs = (long long)__be32_to_cpu(par.pmPyPartStart) * bs;
    			size = (long long)__be32_to_cpu(par.pmPartBlkCnt) * bs;

@@ -196,9 +190,7 @@
    			}
    		}
    	}
-
-	} else {
-		/* Another partition was explicitly requested */
+	} else {	/* Another partition was explicitly requested */
    	SEEK( bs * parnum );
    	READ( &par, sizeof(par) );