Re: [PATCH] Empty if body in linux-record.c

[Hui Zhu <teawater@gmail.com>]

That is great!
Thanks Holger.

Hi Joel,

Do you think I can help this guy check-in this patch directly?

Thanks,
Hui

On Mon, Mar 15, 2010 at 09:00, Joel Brobecker <brobecker@adacore.com> wrote:
>> this is one the issues reported by "clang --analyze" and it appears to
>> me that the code was broken in the same way since the initial check-in
>> of these routines, looking at the tests in gdb.record it does not appear
>> that these are covered at all.
>
> I think you are right. Hui should be able to confirm that the code
> was indeed broken and that the trailing semicolon was not intended.
>
>> I'm not really known to the patch contribution of gdb so I'm not sure if I'm
>> following the proper process here...anyway here is a patch with a
>> ChangeLog entry.
>
> Pretty good for a first try :). Just a couple of comments:
>
>  1. You should indicate how you tested this. Normally, you run
>     the testsuite before and after, and compare the associated gdb.sum
>     file, to make sure that you did not introduce any regression.
>
>     In this particular case, you also need to make sure that process
>     record testing is activated, which it is not by default.
>     Please have a look at
>     http://sourceware.org/gdb/wiki/ProcessRecord#Testing.
>
>   2. Your ChangeLog entry is much too verbose:
>
>>       * linux-record.c (record_linux_msghdr): Fix a compiler warning
>>       about an empty if body that has existed since the addition of the
>>       code. This change will change the return value of the function from
>>       always returning -1 to returning -1 only if calling record_arch_list_add_mem
>>       is failing. The inner for loop will also be iterated more than once
>>       now.
>
>      You just need to say *what* you've done, not why or how.
>      This is explained in more details in the Gnu Coding Standards
>      manual. (I sometimes indulge myself, and add an explaination,
>      but it's because it's a very very short one)
>
>      On possibility would be:
>
>      * linux-record.c (record_linux_msghdr): Remove unintended semicolons.
>
> Would you like write access to the GDB repository?
>
> --
> Joel
>