[haiku-commits] Re: r41343 - haiku/trunk/src/apps/powerstatus

  • From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 06 May 2011 19:17:35 +0200

kallisti5@xxxxxxxxxxx wrote:
> Log:
> add (slightly overly) cautious checks of battery info to avoid NULL 
> segfault... 
> acpi can be flakey at times

Ouch!

> -             && !strcmp(fBatteryExtendedInfo.model_number, "")
> -             && !strcmp(fBatteryExtendedInfo.serial_number, "")
> -             && !strcmp(fBatteryExtendedInfo.type, "")
> -             && !strcmp(fBatteryExtendedInfo.oem_info, ""))
> +             && (!fBatteryExtendedInfo.model_number
> +                     || !strlen(fBatteryExtendedInfo.model_number))
> +             && (!fBatteryExtendedInfo.serial_number
> +                     || !strlen(fBatteryExtendedInfo.serial_number))
> +             && (!fBatteryExtendedInfo.type
> +                     || !strlen(fBatteryExtendedInfo.type))
> +             && (!fBatteryExtendedInfo.oem_info
> +                     || !strlen(fBatteryExtendedInfo.oem_info)))

This is really poor, please revert!
1) !strlen() is no substitute to recognize an empty string -- if that 
string is 60k long, you would count each byte everytime just to see 
that it's not empty.
2) as Rene already pointed out, and you should definitely know before 
doing any changes like this, the fields are character arrays, and can't 
be null! Besides, prefer to check for != NULL instead.

Bye,
   Axel.


Other related posts: