Re: [PATCH] arch/sparc: additional len check in loop for prom_getbootargs

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

 



Hi Cheng.
On Thu, Nov 08, 2012 at 11:41:39AM +0800, Chen Gang wrote:
> 
>   when cp >= barg_buf + BARG_LEN-2, it only break internel loop (while)
>   but outside loop (for) still has effect, and "*cp++ = ' '" repeating
>   so need additional checking for it.
> 
> 
> Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx>

I wonder how you found this bug?!?!
Anyway please consider this alternative fix:

diff --git a/arch/sparc/prom/bootstr_32.c b/arch/sparc/prom/bootstr_32.c
index f5ec32e..4ce602f 100644
--- a/arch/sparc/prom/bootstr_32.c
+++ b/arch/sparc/prom/bootstr_32.c
@@ -31,14 +31,10 @@ prom_getbootargs(void)
 			arg = (*(romvec->pv_v0bootargs))->argv[iter];
 			if (arg == NULL)
 				break;
-			while(*arg != 0) {
-				/* Leave place for space and null. */
-				if(cp >= barg_buf + BARG_LEN-2){
-					/* We might issue a warning here. */
-					break;
-				}
+			while (*arg != 0 && cp < (barg_buf + BARG_LEN - 2))
 				*cp++ = *arg++;
-			}
+
+			/* Append trailing space + null */
 			*cp++ = ' ';
 		}
 		*cp = 0;


Adding the conditional inside the while loop makes
the logic simpler. And the patch actually deletes more lines than it adds.
And please take care to follow coding style too. In particular spaces around operators.

The old code does not follow coding style - but this is no excuse.

Note - the above is not even build tested!

If you use the above code-snippet you can add my:
Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>

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


[Index of Archives]     [Kernel Development]     [DCCP]     [Linux ARM Development]     [Linux]     [Photo]     [Yosemite Help]     [Linux ARM Kernel]     [Linux SCSI]     [Linux x86_64]     [Linux Hams]

  Powered by Linux