On 14-10-08 04:52 PM, Aaro Koskinen wrote:
> Hi,
> 
> On Wed, Oct 08, 2014 at 09:46:55PM +0200, Roberto Medina wrote:
>> Thank you very much for your feedback. I just want to let you know that I
>> didn't ignore that annotation from the last patch. I actually added the
>> white line because checkpatch shows a warning there.
>>
>> WARNING: Missing a blank line after declarations
>> #553: FILE: drivers/staging/octeon/ethernet-tx.c:553:
>> +    cvmx_wqe_t *work = cvmx_fpa_alloc(CVMX_FPA_WQE_POOL);
>> +    if (unlikely(work == NULL)) {
>>
>> I don't see why I shouldn't insert a line there.
> 
> Maybe something like this would be more readable:
> 
>       void *copy_location;
> +     cvmx_wqe_t *work;
>  
>       /* Get a work queue entry */
> -     cvmx_wqe_t *work = cvmx_fpa_alloc(CVMX_FPA_WQE_POOL);
> +     work = cvmx_fpa_alloc(CVMX_FPA_WQE_POOL);
>       if (unlikely(work == NULL)) {
> 
> Then declarations would be correctly separated from the code...

It probably wouldn't hurt -- what I failed to notice when giving
it a quick scan was that it was a clunky typedef in use to create
the variable declaration vs. a sane "struct blah_work *work = ...."
so yes, checkpatch was right in this case.

Paul.
--
> 
> A.
> 
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to