All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] proc/fd: Remove the initialization of variables in seq_show()
@ 2020-05-24  7:46 ` Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-24  7:46 UTC (permalink / raw)
  To: Kaitao Cheng, linux-fsdevel
  Cc: linux-kernel, kernel-janitors, Alexey Dobriyan, Muchun Song

> The variables{files, file} will definitely be assigned,

I find an other specification nicer for these identifiers.


> so we don't need to initialize them.

I suggest to recheck programming concerns around the handling
of the null pointer for the variable “file”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n20
https://elixir.bootlin.com/linux/v5.7-rc6/source/fs/proc/fd.c#L20

Will another imperative wording be preferred for the change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n151

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] proc/fd: Remove the initialization of variables in seq_show()
@ 2020-05-24  7:46 ` Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-24  7:46 UTC (permalink / raw)
  To: Kaitao Cheng, linux-fsdevel
  Cc: linux-kernel, kernel-janitors, Alexey Dobriyan, Muchun Song

> The variables{files, file} will definitely be assigned,

I find an other specification nicer for these identifiers.


> so we don't need to initialize them.

I suggest to recheck programming concerns around the handling
of the null pointer for the variable “file”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n20
https://elixir.bootlin.com/linux/v5.7-rc6/source/fs/proc/fd.c#L20

Will another imperative wording be preferred for the change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n151

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] proc/fd: Remove the initialization of variables in seq_show()
  2020-05-24  7:46 ` Markus Elfring
@ 2020-05-28 12:51   ` Tao pilgrim
  -1 siblings, 0 replies; 6+ messages in thread
From: Tao pilgrim @ 2020-05-28 12:51 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-fsdevel, linux-kernel, kernel-janitors, Alexey Dobriyan,
	Muchun Song

> > The variables{files, file} will definitely be assigned,
>
> I find an other specification nicer for these identifiers.
>
>
> > so we don't need to initialize them.
>
> I suggest to recheck programming concerns around the handling
> of the null pointer for the variable “file”.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n20
> https://elixir.bootlin.com/linux/v5.7-rc6/source/fs/proc/fd.c#L20

We don't need to initialize the variable “file”.
On line 34, if (files) is true,
{file = fcheck_files(files, fd)} will be executed on line 38.
On line 34, if (files) is flse,
{return ret;} will be executed on line 54, and seq_show() will exit directly.
I don't find the programming concerns around the handling of the null
pointer for the variable “file”.

If you have other suggestions, please elaborate on the details.

--
Yours,
Kaitao Cheng

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] proc/fd: Remove the initialization of variables in seq_show()
@ 2020-05-28 12:51   ` Tao pilgrim
  0 siblings, 0 replies; 6+ messages in thread
From: Tao pilgrim @ 2020-05-28 12:51 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-fsdevel, linux-kernel, kernel-janitors, Alexey Dobriyan,
	Muchun Song

> > The variables{files, file} will definitely be assigned,
>
> I find an other specification nicer for these identifiers.
>
>
> > so we don't need to initialize them.
>
> I suggest to recheck programming concerns around the handling
> of the null pointer for the variable “file”.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=caffb99b6929f41a69edbb5aef3a359bf45f3315#n20
> https://elixir.bootlin.com/linux/v5.7-rc6/source/fs/proc/fd.c#L20

We don't need to initialize the variable “file”.
On line 34, if (files) is true,
{file = fcheck_files(files, fd)} will be executed on line 38.
On line 34, if (files) is flse,
{return ret;} will be executed on line 54, and seq_show() will exit directly.
I don't find the programming concerns around the handling of the null
pointer for the variable “file”.

If you have other suggestions, please elaborate on the details.

--
Yours,
Kaitao Cheng

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: proc/fd: Remove the initialization of variables in seq_show()
  2020-05-28 12:51   ` Tao pilgrim
@ 2020-05-28 16:04     ` Markus Elfring
  -1 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-28 16:04 UTC (permalink / raw)
  To: Tao pilgrim, linux-fsdevel
  Cc: linux-kernel, kernel-janitors, Alexey Dobriyan, Muchun Song

>>> The variables{files, file} will definitely be assigned,
>>
>> I find an other specification nicer for these identifiers.
>>
>>
>>> so we don't need to initialize them.
> We don't need to initialize the variable “file”.

I can agree to this interpretation of the software situation
because there is a precondition involved for the variable “ret”.
https://elixir.bootlin.com/linux/v5.7-rc7/source/fs/proc/fd.c#L20
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=b0c3ba31be3e45a130e13b278cf3b90f69bda6f6#n20


> I don't find the programming concerns around the handling of the null
> pointer for the variable “file”.

I find the initial change description too terse and therefore incomplete.


> If you have other suggestions, please elaborate on the details.

I propose to extend the patch.
How do you think about to convert initialisations for the variables
“f_flags” and “ret” also into later assignments?

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: proc/fd: Remove the initialization of variables in seq_show()
@ 2020-05-28 16:04     ` Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-05-28 16:04 UTC (permalink / raw)
  To: Tao pilgrim, linux-fsdevel
  Cc: linux-kernel, kernel-janitors, Alexey Dobriyan, Muchun Song

>>> The variables{files, file} will definitely be assigned,
>>
>> I find an other specification nicer for these identifiers.
>>
>>
>>> so we don't need to initialize them.
> We don't need to initialize the variable “file”.

I can agree to this interpretation of the software situation
because there is a precondition involved for the variable “ret”.
https://elixir.bootlin.com/linux/v5.7-rc7/source/fs/proc/fd.c#L20
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/proc/fd.c?id=b0c3ba31be3e45a130e13b278cf3b90f69bda6f6#n20


> I don't find the programming concerns around the handling of the null
> pointer for the variable “file”.

I find the initial change description too terse and therefore incomplete.


> If you have other suggestions, please elaborate on the details.

I propose to extend the patch.
How do you think about to convert initialisations for the variables
“f_flags” and “ret” also into later assignments?

Regards,
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-28 16:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-24  7:46 [PATCH] proc/fd: Remove the initialization of variables in seq_show() Markus Elfring
2020-05-24  7:46 ` Markus Elfring
2020-05-28 12:51 ` Tao pilgrim
2020-05-28 12:51   ` Tao pilgrim
2020-05-28 16:04   ` Markus Elfring
2020-05-28 16:04     ` Markus Elfring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.