[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
> <programmingkidx at gmail.com> 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 <programmingkidx at gmail.com>
>> 
>> 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 <programmingkidx at gmail.com>

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) );
 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openbios.org/pipermail/openbios/attachments/20121204/8193959e/attachment-0001.html>