Re: [Qemu-block] [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

On Feb 2, 2016, at 2:24 PM, Eric Blake wrote:

> On 02/02/2016 12:10 PM, Programmingkid wrote:
> 
>>> There was/is no leak because it qdict_get_str() returns 'const char *' and
>>> so nothing needs freeing. So your change is still a backwards steps IMHO.
>> 
>> char filename[MAXPATHLEN];
>> snprintf(filename, MAXPATHLEN, "%s", qdict_get_str(options, "filename"));
>> 
>> So you want the above to be changed so that it goes back to this:
>> 
>> const char *filename = qdict_get_str(options, "filename");
> 
> Correct.
> 
>> 
>> then this code would have to be changed:
>> 
>> snprintf(filename, MAXPATHLEN, "%s", bsd_path);
>> qdict_put(options, "filename", qstring_from_str(filename));
>> 
>> I could change it to this:
>> 
>> qdict_put(options, "filename", qstring_from_str(bsd_path));
> 
> Correct.
> 
>> 
>> If I did that, then this part would not be accurate anymore:
>> 
>> if (strncmp(filename, "/dev/", 5) == 0) {
>>            print_unmounting_directions(filename);
>>            return -1;
>>        }
>> 
>> filename would be just "/dev/cdrom" for when the user uses the optical 
>> drive. This would print incorrect unmounting directions. 
> 
> So fix it to call print_unmounting_directions(bsd_path), after hoisting
> the declaration of bsd_path to be earlier to have a long enough scope.
> 
>> 
>> I could add another variable that keeps track of the real device name, but 
>> that would consume more memory. It is so much easier and simpler to just use 
>> the fixed array.
> 
> And why isn't bsd_path usable for that purpose?

After trying it out, I found out why bsd_path isn't usable for that purpose. It 
is because the user might try to use a flash drive as the the cdrom. Say a 
flash drive is set to /dev/disk2s9. If the user issues the monitor command 
"change ide1-cd0 /dev/disk2s9", this will make "if (strcmp(filename, 
"/dev/cdrom") == 0)" false and bsd_path would never be set. bsd_path contents 
would be garbage.

This would lead to this code not printing the unmounting directions:

if (strncmp(filename, "/dev/", 5) == 0) {
           print_unmounting_directions(filename);
           return -1;
       }

It looks keeping filename as an character array is best.