From: Ozgur Karatas <okaratas@member.fsf.org>
To: Eric Sandeen <sandeen@sandeen.net>, Joe Perches <joe@perches.com>,
"david@fromorbit.com" <david@fromorbit.com>
Cc: "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] Fixed to codestyle
Date: Mon, 12 Dec 2016 22:34:01 +0200 [thread overview]
Message-ID: <737751481574841@web26j.yandex.ru> (raw)
In-Reply-To: <50420874-ffde-283f-f2e7-cb2ebbec1b84@sandeen.net>
12.12.2016, 20:35, "Eric Sandeen" <sandeen@sandeen.net>:
> 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@member.fsf.org>
>>>
>>> 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
next prev parent reply other threads:[~2016-12-12 20:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-12 10:53 [PATCH 1/1] Fixed to codestyle Ozgur Karatas
2016-12-12 13:49 ` Eric Sandeen
2016-12-12 18:14 ` Joe Perches
2016-12-12 18:35 ` Eric Sandeen
2016-12-12 20:34 ` Ozgur Karatas [this message]
2016-12-12 20:41 ` Eric Sandeen
2016-12-12 20:49 ` Ozgur Karatas
2016-12-12 20:50 ` Joe Perches
2016-12-12 20:54 ` Ozgur Karatas
2016-12-12 21:17 ` Eric Sandeen
2016-12-12 20:31 ` Ozgur Karatas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=737751481574841@web26j.yandex.ru \
--to=okaratas@member.fsf.org \
--cc=david@fromorbit.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox