Re: [Qemu-devel] [Qemu-ppc] [PATCH] Revert "Update OpenBIOS images"

[Alexander Graf]

On 14.02.2013, at 01:54, Mark Cave-Ayland wrote:

> On 14/02/13 00:17, Alexander Graf wrote:
> 
>> With the following patch fixing the issue at hand for me. Though I don't 
>> fully understand why str would be NULL yet:
>> 
>> 
>> diff --git a/packages/mac-parts.c b/packages/mac-parts.c
>> index a286870..443455e 100644
>> --- a/packages/mac-parts.c
>> +++ b/packages/mac-parts.c
>> @@ -140,7 +140,7 @@ macparts_open( macparts_info_t *di )
>>          * Implement partition selection as per the PowerPC Microprocessor 
>> CHRP bindings
>>          */
>> 
>> -       if (parnum == 0) {
>> +       if (str == NULL || parnum == 0) {
>>                 /* According to the spec, partition 0 as well as no 
>> arguments means the whole disk */
>>                 offs = (long long)0;
>>                 size = (long long)__be32_to_cpu(dmap.sbBlkCount) * bs;
>> 
>> Alex
> 
> Ah okay. It's actually caused by this bit of logic in libopenbios/bindings.c 
> and assuming that my_args() is a zero length Forth string:
> 
> char *
> pop_fstr_copy( void )
> {
>        int len = POP();
>        char *str, *p = (char*)cell2pointer(POP());
>        if( !len )
>                return NULL;
>        str = malloc( len + 1 );
>        if( !str )
>                return NULL;
>        memcpy( str, p, len );
>        str[len] = 0;
>        return str;
> }
> 
> The check for a zero length string and returning NULL has caused me problems 
> before when round-tripping strings between Forth and C.
> 
> Without testing the patch myself, I'd say that it looks good. I can run it 
> over my complete set of test images tomorrow evening if that would be 
> acceptable?

Either we get the fix in in the next few hours (and thus agree that it's the 
right way forward) or there's no rush - QEMU 1.5 is quite a while to go.

> Can you post a git diff version to the OpenBIOS mailing list too?

Sure!


Alex