[haiku-commits] Re: r41343 - haiku/trunk/src/apps/powerstatus
- From: "Ingo Weinhold" <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 06 May 2011 19:51:38 +0200
-------- Original-Nachricht --------
> Datum: Fri, 06 May 2011 19:17:35 +0200
> Von: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
> An: haiku-commits@xxxxxxxxxxxxx
> Betreff: [haiku-commits] Re: r41343 - haiku/trunk/src/apps/powerstatus
> 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.
More generally I'd recommend explicitly using bool expressions where bool
expressions are expected instead of the implicit conversions from
integer/pointer expressions ("strlen(...) >/!= 0" is so much clearer).
BTW a cheap check for an empty string is "string[0] == '\0'" or "*string ==
'\0'". Though I wouldn't rule out that the compiler optimizes "strlen(string)
== 0" or "strcmp(string, "") == 0" to the same target code.
CU, Ingo
Other related posts:
