Re: [PATCH 1/1] Fixed to codestyle

From: Ozgur Karatas
Date: Mon Dec 12 2016 - 15:31:20 EST


12.12.2016, 20:14, "Joe Perches" <joe@xxxxxxxxxxx>:
> 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>

Dear Joe;
Firstly, I have studied and so I thought it was correct to use it as (* uuid) in this regard Mr. Eric corrected me and he is explanatory to me.

fs/xfs/uuid.c:21: WARNING: do not add new typedefs
fs/xfs/uuid.c:36: ERROR: space prohibited before open square bracket '['
fs/xfs/uuid.c:54: WARNING: sizeof *uuid should be sizeof(*uuid)
fs/xfs/uuid.c:55: ERROR: trailing statements should be on next line
total: 2 errors, 2 warnings, 63 lines checked

>> Â> 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.

You can see below:
fs/xfs/uuid.c:54: WARNING: sizeof *uuid should be sizeof(*uuid)
Regards,

Ozgur

>> Â> return 1; /* is nil */
>> Â> }
>> Â>