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.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
