Re: [PATCH 1/1] Fixed to codestyle

From: Ozgur Karatas
Date: Mon Dec 12 2016 - 15:34:10 EST


12.12.2016, 20:35, "Eric Sandeen" <sandeen@xxxxxxxxxxx>:
> On 12/12/16 12:14 PM, Joe Perches wrote:
>> ÂOn Mon, 2016-12-12 at 07:49 -0600, Eric Sandeen wrote:
>>> ÂOn 12/12/16 4:53 AM, Ozgur Karatas wrote:
>>>> ÂHello,
>>>>
>>>> ÂI have error to use uuid and I think the functions should be used when -i'm eye-catching- "(* uuid)".
>>>> ÂI tested it.
>>>>
>>>> ÂRegards,
>>>>
>>>> ÂSigned-off-by: Ozgur Karatas <okaratas@xxxxxxxxxxxxxx>
>>>
>>> ÂNAK
>>>
>>> ÂThis doesn't fix code style at all; there is no need and no
>>> Âprecedence for i.e. (*uuid) in function arguments in the xfs code,
>>> Âand you have broken indentation in the loop within the function.
>>
>> ÂPerhaps better would be to convert the xfs uuid_t typedef
>> Âto the include/uapi/linux/uuid.h appropriate struct and
>> Âmaybe use a comparison to NULL_UUID_<type>
>>
>>>> Âdiff --git a/fs/xfs/uuid.c b/fs/xfs/uuid.c
>> Â[]
>>>> Â@@ -33,7 +33,7 @@ typedef struct {
>>>> ÂÂÂ* it just something that's needed for user-level file handles.
>>>> ÂÂÂ*/
>>>> ÂÂvoid
>>>> Â-uuid_getnodeuniq(uuid_t *uuid, int fsid [2])
>>>> Â+uuid_getnodeuniq(uuid_t (*uuid), int fsid [2])
>>
>> ÂAnd to amplify Eric's comment:
>>
>> Âthat bit is confusing as it makes uuid look
>> Âlike a function pointer.
>>
>>>> ÂÂ{
>>>> ÂÂÂÂÂÂÂÂÂÂxfs_uu_t *uup = (xfs_uu_t *)uuid;
>>>>
>>>> Â@@ -51,8 +51,8 @@ uuid_is_nil(uuid_t *uuid)
>>>> ÂÂÂÂÂÂÂÂÂÂif (uuid == NULL)
>>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn 0;
>>>> ÂÂÂÂÂÂÂÂÂÂ/* implied check of version number here... */
>>>> Â- for (i = 0; i < sizeof *uuid; i++)
>>>> Â- if (*cp++) return 0; /* not nil */
>>>> Â+ for (i = 0; i < sizeof (*uuid); i++)
>>>> Â+ if (*cp++) return 0; /* not nil */
>>
>> ÂThere shouldn't be a space after sizeof.
>
> and the "if" /should/ be indented under the for loop, because
> it is within the loop...
>
> I suppose simply:
>
> - for (i = 0; i < sizeof *uuid; i++)
> + for (i = 0; i < sizeof(*uuid); i++)
>
> would be fine on its own, though, because that is a bit
> unusual/inconsistent. I'll admit that I didn't spot
> that change as I scanned over the unnecessary & incorrect parts
> of the first patch. :)
>
> thanks,
> -Eric

Dear Eric;

Can you tell me the true code style? should use to (* uuid)?
I'm learn to new and I'm newbies :)

Sorry,

Regards

Ozgur