Virtualization - May 2019 - [PATCH v3 01/19] drm: Add |struct drm_gem_vram_object| and helpers

Hi Thomas.
> >> +
> >> +/**
> >> + * Returns the container of type &struct drm_gem_vram_object
> >> + * for field bo.
> >> + * @bo:		the VRAM buffer object
> >> + * Returns:	The containing GEM VRAM object
> >> + */
> >> +static inline struct drm_gem_vram_object* drm_gem_vram_of_bo(
> >> +	struct ttm_buffer_object *bo)
> >> +{
> >> +	return container_of(bo, struct drm_gem_vram_object, bo);
> >> +}
> > Indent funny. USe same indent as used in other parts of file for
> > function arguments.
> 
> If I put the argument next to the function's name, it will exceed the
> 80-character limit. From the coding-style document, I could not see what
> to do in this case. One solution would move the return type to a
> separate line before the function name. I've not seen that anywhere in
> the source code, so moving the argument onto a separate line and
> indenting by one tab appears to be the next best solution. Please let me
> know if there's if there's a preferred style for cases like this
one.
Readability has IMO higher priority than some limit of 80 chars.
And it hurts readability (at least my OCD) when style changes
as you do with indent here. So my personal preference is to fix
indent and accect longer lines.

But you ask for a preferred style - which I do not think we have in this
case. So it boils down to what you prefer.

Enough bikeshedding, thanks for the quick response.

	Sam

Am 30.04.19 um 11:23 schrieb Sam Ravnborg:
> [CAUTION: External Email]
>
> Hi Thomas.
>
>>>> +
>>>> +/**
>>>> + * Returns the container of type &struct
drm_gem_vram_object
>>>> + * for field bo.
>>>> + * @bo:           the VRAM buffer object
>>>> + * Returns:       The containing GEM VRAM object
>>>> + */
>>>> +static inline struct drm_gem_vram_object* drm_gem_vram_of_bo(
>>>> +  struct ttm_buffer_object *bo)
>>>> +{
>>>> +  return container_of(bo, struct drm_gem_vram_object, bo);
>>>> +}
>>> Indent funny. USe same indent as used in other parts of file for
>>> function arguments.
>> If I put the argument next to the function's name, it will exceed
the
>> 80-character limit. From the coding-style document, I could not see
what
>> to do in this case. One solution would move the return type to a
>> separate line before the function name. I've not seen that anywhere
in
>> the source code, so moving the argument onto a separate line and
>> indenting by one tab appears to be the next best solution. Please let
me
>> know if there's if there's a preferred style for cases like
this one.
> Readability has IMO higher priority than some limit of 80 chars.
> And it hurts readability (at least my OCD) when style changes
> as you do with indent here. So my personal preference is to fix
> indent and accect longer lines.
In this case the an often used convention (which is also kind of 
readable) is to add a newline after the return values, but before the 
function name. E.g. something like this:

static inline struct drm_gem_vram_object*
drm_gem_vram_of_bo(struct ttm_buffer_object *bo)

Regards,
Christian.
>
> But you ask for a preferred style - which I do not think we have in this
> case. So it boils down to what you prefer.
>
> Enough bikeshedding, thanks for the quick response.
>
>          Sam

Hi Christian,

would you review the whole patch set? Daniel mentioned that he'd prefer
to leave the review to memory-mgmt developers.

Best regards
Thomas

Am 30.04.19 um 11:35 schrieb Koenig, Christian:
> Am 30.04.19 um 11:23 schrieb Sam Ravnborg:
>> [CAUTION: External Email]
>>
>> Hi Thomas.
>>
>>>>> +
>>>>> +/**
>>>>> + * Returns the container of type &struct
drm_gem_vram_object
>>>>> + * for field bo.
>>>>> + * @bo:           the VRAM buffer object
>>>>> + * Returns:       The containing GEM VRAM object
>>>>> + */
>>>>> +static inline struct drm_gem_vram_object*
drm_gem_vram_of_bo(
>>>>> +  struct ttm_buffer_object *bo)
>>>>> +{
>>>>> +  return container_of(bo, struct drm_gem_vram_object, bo);
>>>>> +}
>>>> Indent funny. USe same indent as used in other parts of file
for
>>>> function arguments.
>>> If I put the argument next to the function's name, it will
exceed the
>>> 80-character limit. From the coding-style document, I could not see
what
>>> to do in this case. One solution would move the return type to a
>>> separate line before the function name. I've not seen that
anywhere in
>>> the source code, so moving the argument onto a separate line and
>>> indenting by one tab appears to be the next best solution. Please
let me
>>> know if there's if there's a preferred style for cases like
this one.
>> Readability has IMO higher priority than some limit of 80 chars.
>> And it hurts readability (at least my OCD) when style changes
>> as you do with indent here. So my personal preference is to fix
>> indent and accect longer lines.
> 
> In this case the an often used convention (which is also kind of 
> readable) is to add a newline after the return values, but before the 
> function name. E.g. something like this:
> 
> static inline struct drm_gem_vram_object*
> drm_gem_vram_of_bo(struct ttm_buffer_object *bo)
> 
> Regards,
> Christian.
> 
>>
>> But you ask for a preferred style - which I do not think we have in
this
>> case. So it boils down to what you prefer.
>>
>> Enough bikeshedding, thanks for the quick response.
>>
>>          Sam
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG N?rnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20190503/ffe43a7d/attachment.sig>