[PATCH] Empty if body in linux-record.c
Hui Zhu
teawater@gmail.com
Tue Mar 16 08:01:00 GMT 2010
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
>
More information about the Gdb-patches
mailing list