On Thu, Apr 11, 2024 at 08:02:56PM +0200, Roman Storozhenko wrote:
> Add a cleanup function attribute '__free(device_node)' to the device node
> pointer initialization statement and remove the pairing cleanup function
> call of 'of_node_put' at the end of the function.
> The '_free()' attrubute is introduced by scope-based resource management
> in-kernel framework implemented in 'cleanup.h'. A pointer marked with
> '__free()' attribute makes a compiler insert a cleanup function call
> to the places where the pointer goes out of the scope. This feature
> allows to get rid of manual cleanup function calls.
>
> Suggested-by: Julia.Lawall <[email protected]>
> Signed-off-by: Roman Storozhenko <[email protected]>
> ---
> This patch targets the next tree:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> tag: next-20240411
> ---
> drivers/tty/sysrq.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 02217e3c916b..1d1261f618c0 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -758,11 +758,12 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
> static void sysrq_of_get_keyreset_config(void)
> {
> u32 key;
> - struct device_node *np;
> struct property *prop;
> const __be32 *p;
>
> - np = of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> + struct device_node *np __free(device_node) =
> + of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> +
Did you run this through checkpatch.pl? Please do so.
Hi Greg,
This is the output of the checkpatch:
hedin@laptop:~/prj/linux-next$ ./scripts/checkpatch.pl --strict
~/lkmp/course_tasks/coccinele/patches/sysrq/v1/*
--------------------------------------------------------------------------------
/home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0000-cover-letter.patch
--------------------------------------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 0 lines checked
/home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0000-cover-letter.patch
has no obvious style problems and is ready for submission.
------------------------------------------------------------------------------------------------------------------------
/home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0001-sysrq-Auto-release-device-node-using-__free-attribut.patch
------------------------------------------------------------------------------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 16 lines checked
/home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0001-sysrq-Auto-release-device-node-using-__free-attribut.patch
has no obvious style problems and is ready for submission.
Before sending this patch to the mailing list I sent it to me,
downloaded from the email, and then applied atop of the tree using git
am. Encountered no issues.
I can guess that you checked the whole file and got some issues. But
those are not related to my changes.
Thanks,
Roman
On Thu, Apr 11, 2024 at 8:10 PM Greg KH <[email protected]> wrote:
>
> On Thu, Apr 11, 2024 at 08:02:56PM +0200, Roman Storozhenko wrote:
> > Add a cleanup function attribute '__free(device_node)' to the device node
> > pointer initialization statement and remove the pairing cleanup function
> > call of 'of_node_put' at the end of the function.
> > The '_free()' attrubute is introduced by scope-based resource management
> > in-kernel framework implemented in 'cleanup.h'. A pointer marked with
> > '__free()' attribute makes a compiler insert a cleanup function call
> > to the places where the pointer goes out of the scope. This feature
> > allows to get rid of manual cleanup function calls.
> >
> > Suggested-by: Julia.Lawall <[email protected]>
> > Signed-off-by: Roman Storozhenko <[email protected]>
> > ---
> > This patch targets the next tree:
> > tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > tag: next-20240411
> > ---
> > drivers/tty/sysrq.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> > index 02217e3c916b..1d1261f618c0 100644
> > --- a/drivers/tty/sysrq.c
> > +++ b/drivers/tty/sysrq.c
> > @@ -758,11 +758,12 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
> > static void sysrq_of_get_keyreset_config(void)
> > {
> > u32 key;
> > - struct device_node *np;
> > struct property *prop;
> > const __be32 *p;
> >
> > - np = of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> > + struct device_node *np __free(device_node) =
> > + of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> > +
>
> Did you run this through checkpatch.pl? Please do so.
>
--
Kind regards,
Roman Storozhenko
On Thu, Apr 11, 2024 at 08:25:10PM +0200, Roman Storozhenko wrote:
> Hi Greg,
>
> This is the output of the checkpatch:
> hedin@laptop:~/prj/linux-next$ ./scripts/checkpatch.pl --strict
> ~/lkmp/course_tasks/coccinele/patches/sysrq/v1/*
> --------------------------------------------------------------------------------
> /home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0000-cover-letter.patch
> --------------------------------------------------------------------------------
> total: 0 errors, 0 warnings, 0 checks, 0 lines checked
>
> /home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0000-cover-letter.patch
> has no obvious style problems and is ready for submission.
> ------------------------------------------------------------------------------------------------------------------------
> /home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0001-sysrq-Auto-release-device-node-using-__free-attribut.patch
> ------------------------------------------------------------------------------------------------------------------------
> total: 0 errors, 0 warnings, 0 checks, 16 lines checked
>
> /home/hedin/lkmp/course_tasks/coccinele/patches/sysrq/v1/0001-sysrq-Auto-release-device-node-using-__free-attribut.patch
> has no obvious style problems and is ready for submission.
>
> Before sending this patch to the mailing list I sent it to me,
> downloaded from the email, and then applied atop of the tree using git
> am. Encountered no issues.
>
> I can guess that you checked the whole file and got some issues. But
> those are not related to my changes.
>
> Thanks,
> Roman
>
> On Thu, Apr 11, 2024 at 8:10 PM Greg KH <[email protected]> wrote:
> >
> > On Thu, Apr 11, 2024 at 08:02:56PM +0200, Roman Storozhenko wrote:
> > > Add a cleanup function attribute '__free(device_node)' to the device node
> > > pointer initialization statement and remove the pairing cleanup function
> > > call of 'of_node_put' at the end of the function.
> > > The '_free()' attrubute is introduced by scope-based resource management
> > > in-kernel framework implemented in 'cleanup.h'. A pointer marked with
> > > '__free()' attribute makes a compiler insert a cleanup function call
> > > to the places where the pointer goes out of the scope. This feature
> > > allows to get rid of manual cleanup function calls.
> > >
> > > Suggested-by: Julia.Lawall <[email protected]>
> > > Signed-off-by: Roman Storozhenko <[email protected]>
> > > ---
> > > This patch targets the next tree:
> > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > > tag: next-20240411
> > > ---
> > > drivers/tty/sysrq.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> > > index 02217e3c916b..1d1261f618c0 100644
> > > --- a/drivers/tty/sysrq.c
> > > +++ b/drivers/tty/sysrq.c
> > > @@ -758,11 +758,12 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
> > > static void sysrq_of_get_keyreset_config(void)
> > > {
> > > u32 key;
> > > - struct device_node *np;
> > > struct property *prop;
> > > const __be32 *p;
> > >
> > > - np = of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> > > + struct device_node *np __free(device_node) =
> > > + of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> > > +
> >
> > Did you run this through checkpatch.pl? Please do so.
The issue is the blank line in the variable list now. Odd that
checkpatch doesn't catch it, which implies the complexity added here
might just not be a good idea :(
thanks,
greg k-h