On 07/12/2020 11.26, BALATON Zoltan via wrote:
> On Mon, 7 Dec 2020, Alexey Kardashevskiy wrote:
>> On 05/12/2020 05:32, Greg Kurz wrote:
>>> On Tue, 13 Oct 2020 13:19:11 +1100
>>> Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
>>>
>>>> +static void readstr(hwaddr pa, char *buf, int size)
>>>> +{
>>>> + cpu_physical_memory_read(pa, buf, size);
>>>> + if (buf[size - 1] != '\0') {
>>>> + buf[size - 1] = '\0';
>>>> + if (strlen(buf) == size - 1) {
>>>> + trace_spapr_of_client_error_str_truncated(buf, size);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +static bool cmpservice(const char *s, size_t len,
>>>> + unsigned nargs, unsigned nret,
>>>> + const char *s1, size_t len1,
>>>> + unsigned nargscheck, unsigned nretcheck)
>>>> +{
>>>> + if (strcmp(s, s1)) {
>>>> + return false;
>>>> + }
>>>> + if ((nargscheck && (nargs != nargscheck)) ||
>>>> + (nretcheck && (nret != nretcheck))) {
>>>
>>> Parenthesitis : != has precedence over &&.
>>
>>
>> I love my braces :)
>
> Then keep them for yourself and not leave them around in code :-) I prefer
> minimum parenthesis too as that's easier to read and you can always look up
> operator precedence if in doubt so I'd vote for dropping them unless really
> needed or the compiler complains about it.
+1
Too many braces always rather confuse me than being helpful.
>>>> +static uint32_t of_client_setprop(SpaprMachineState *spapr,
>>>> + uint32_t nodeph, uint32_t pname,
>>>> + uint32_t valaddr, uint32_t vallen)
>>>> +{
>>>> + char propname[OF_PROPNAME_LEN_MAX + 1];
>>>> + uint32_t ret = -1;
>>>> + int offset;
>>>> + char trval[64] = "";
>>>> +
>>>> + readstr(pname, propname, sizeof(propname));
>>>> + /*
>>>> + * We only allow changing properties which we know how to update on
>>>> + * the QEMU side.
>>>> + */
>>>> + if (vallen == sizeof(uint32_t)) {
>>>> + uint32_t val32 = ldl_be_phys(first_cpu->as, valaddr);
>>>> +
>>>> + if ((strcmp(propname, "linux,rtas-base") == 0) ||
>>>> + (strcmp(propname, "linux,rtas-entry") == 0)) {
>>>
>>> What about :
>>>
>>> if (!strcmp(propname, "linux,rtas-base") ||
>>> !strcmp(propname, "linux,rtas-entry")) {
>>
>>
>>
>> [fstn1-p1 qemu-killslof]$ git grep 'strcmp.*==.*0' | wc -l
>> 426
>> [fstn1-p1 qemu-killslof]$ git grep '!strcmp' | wc -l
>> 447
>>
>> My team is losing but not by much :) I prefer "==" (literally - "equal) to
>> "!" with "cmp" which is "does not compare" (makes little sense).
>
> This may be personal preference but I also prefer using !strcmp for match
> (and less parenthesis :-) )
Easy solution: Simply use the glib variant g_str_equal() instead - that's
way much easier to understand while reading the code!
Thomas
