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.