Re: [linux-safety] [PATCH] parse-sublevel-option.h : Add new line and license identifier
"Nghia Le" <[email protected]> Thu, 19 Aug 2021 17:01:31 +0700
| Newsgroups | tech.elisa.lists.linux-safety |
|---|---|
| Message-ID | <CADkY59uq24e4=F3r=Zz9NGtzQfvZp0TM3Yk3-ijT1Pt8y_ex1Q@mail.gmail.com> |
--0000000000003fda5405c9e6a3d4 Content-Type: text/plain; charset="UTF-8" On Thu, Aug 19, 2021 at 12:11 AM Lukas Bulwahn <[email protected]> wrote: > On Wed, Aug 18, 2021 at 7:01 PM Nghia Le <[email protected]> wrote: > > > > Add the new line at the end of file and add SPDX license identifier > GPL-2.0, thus removing warning while running checkpatch.pl script > > > > Signed-off-by: Nghia Le <[email protected]> > > Did you run checkpatch.pl on your own patch? What did it report? > I will add report in the PATCH v2 > --- > > tools/perf/util/parse-sublevel-options.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/parse-sublevel-options.h > b/tools/perf/util/parse-sublevel-options.h > > index 9b9efcc2aaad..bb44b5af2963 100644 > > --- a/tools/perf/util/parse-sublevel-options.h > > +++ b/tools/perf/util/parse-sublevel-options.h > > @@ -1,3 +1,5 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > Do you understand the legal implications of adding this annotation? > Are you among the authors to make this statement with sufficient > confidence? > > It seems awkward to just annotate one file here, probably it is much > better to have one commit that adds this annotation to all files in > tools/perf/ if you are sure that this is all licensed GPL-2.0. > Thanks for your comment. Originally, I had two reasons to add license identifier 1. To remove the warning, I referred lots of header file in the same folder /tools/perf/util and and many of them have license identifier with SPDX is GPL-2.0. Then I thought we can apply for file which doesn't have identifier explicitly 2. I also refer the topic with long conversation while Greg K-H mentioned about add SPDX for file without license identifier: https://patchwork.kernel.org/project/xfs/patch/20171107020607.GA26910@magnolia/ However, I totally agree with you that it is quite complicated in terms of legal implication and I am not confident enough to change it at the moment. Hence, I will keep it as original version > #ifndef _PERF_PARSE_SUBLEVEL_OPTIONS_H > > #define _PERF_PARSE_SUBLEVEL_OPTIONS_H > > > > @@ -8,4 +10,5 @@ struct sublevel_option { > > > > int perf_parse_sublevel_options(const char *str, struct sublevel_option > *opts); > > > > -#endif > > \ No newline at end of file > > +#endif > > + > > Now, you are adding an empty new line to this file. Can you create a > patch that ends with #endif and a newline instead? That seems much > more common throughout the whole kernel tree. > I will update in PATCH v2 Good luck, we are anxiously waiting for your patch v2. > > Lukas > Thank you for your detailed comments Nghia Le -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#242): https://lists.elisa.tech/g/linux-safety/message/242 Mute This Topic: https://lists.elisa.tech/mt/84977020/5278000 Group Owner: [email protected] Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=- --0000000000003fda5405c9e6a3d4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Thu, Aug 19, 2021 at 12:11 AM Luka= s Bulwahn <<a href=3D"mailto:[email protected]">lukas.bulwahn@gmai= l.com</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"ma= rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:= 1ex">On Wed, Aug 18, 2021 at 7:01 PM Nghia Le <<a href=3D"mailto:nghialm= [email protected]" target=3D"_blank">[email protected]</a>> wrote:<br> ><br> > Add the new line at the end of file and add SPDX license identifier GP= L-2.0, thus removing warning while running <a href=3D"http://checkpatch.pl"= rel=3D"noreferrer" target=3D"_blank">checkpatch.pl</a> script<br> ><br> > Signed-off-by: Nghia Le <<a href=3D"mailto:[email protected]" tar= get=3D"_blank">[email protected]</a>><br> <br> Did you run <a href=3D"http://checkpatch.pl" rel=3D"noreferrer" target=3D"_= blank">checkpatch.pl</a> on your own patch? What did it report?<br></blockq= uote><div>=C2=A0 =C2=A0=C2=A0</div><div>=C2=A0 I=C2=A0 will add report in t= he PATCH v2</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"= margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-lef= t:1ex"> > ---<br> >=C2=A0 tools/perf/util/parse-sublevel-options.h | 5 ++++-<br> >=C2=A0 1 file changed, 4 insertions(+), 1 deletion(-)<br> ><br> > diff --git a/tools/perf/util/parse-sublevel-options.h b/tools/perf/uti= l/parse-sublevel-options.h<br> > index 9b9efcc2aaad..bb44b5af2963 100644<br> > --- a/tools/perf/util/parse-sublevel-options.h<br> > +++ b/tools/perf/util/parse-sublevel-options.h<br> > @@ -1,3 +1,5 @@<br> > +/* SPDX-License-Identifier: GPL-2.0 */<br> > +<br> <br> Do you understand the legal implications of adding this annotation?<br> Are you among the authors to make this statement with sufficient<br> confidence?<br> <br> It seems awkward to just annotate one file here, probably it is much<br> better to have one commit that adds this annotation to all files in<br> tools/perf/ if you are sure that this is all licensed GPL-2.0.<br></blockqu= ote><div>=C2=A0</div><div>=C2=A0 Thanks for your comment. Originally, I had= two reasons to add license identifier</div><div>=C2=A0 1. To remove the wa= rning, I referred lots of header file in the same folder /tools/perf/util a= nd=C2=A0</div><div>=C2=A0 =C2=A0 =C2=A0 and many of them have license ident= ifier with SPDX is GPL-2.0. Then I thought we can apply for file which</div= ><div>=C2=A0 =C2=A0 =C2=A0 doesn't have identifier explicitly</div><div= >=C2=A0 2. I also refer the topic with long conversation while Greg K-H men= tioned about add SPDX for file without license</div><div>=C2=A0 =C2=A0 =C2= =A0 identifier:=C2=A0 <a href=3D"https://patchwork.kernel.org/project/xfs/p= atch/20171107020607.GA26910@magnolia/">https://patchwork.kernel.org/project= /xfs/patch/20171107020607.GA26910@magnolia/</a></div><div><br></div><div>= =C2=A0 =C2=A0However, I totally agree with you that it is quite complicated= in terms of legal implication and I am not confident enough=C2=A0</div><di= v>=C2=A0 =C2=A0to change it at the moment. Hence, I will keep it as origina= l version</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"ma= rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:= 1ex"> >=C2=A0 #ifndef _PERF_PARSE_SUBLEVEL_OPTIONS_H<br> >=C2=A0 #define _PERF_PARSE_SUBLEVEL_OPTIONS_H<br> ><br> > @@ -8,4 +10,5 @@ struct sublevel_option {<br> ><br> >=C2=A0 int perf_parse_sublevel_options(const char *str, struct sublevel= _option *opts);<br> ><br> > -#endif<br> > \ No newline at end of file<br> > +#endif<br> > +<br> <br> Now, you are adding an empty new line to this file. Can you create a<br> patch that ends with #endif and a newline instead? That seems much<br> more common throughout the whole kernel tree.<br></blockquote><div>=C2=A0 = =C2=A0 =C2=A0 =C2=A0=C2=A0</div><div>=C2=A0 =C2=A0 I will update in PATCH v= 2</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px= 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Good luck, we are anxiously waiting for your patch v2.<br> <br> Lukas<br></blockquote><div>=C2=A0 =C2=A0 Thank you for your detailed commen= ts</div><div><br></div><div>Nghia Le=C2=A0</div></div></div> <div width=3D"1" style=3D"color:white;clear:both">_._,_._,_</div> <hr> L= inks:<p> You receive all messages sent to this group. <p> <a target=3D= "_blank" href=3D"https://lists.elisa.tech/g/linux-safety/message/242">View/= Reply Online (#242)</a> | <a target=3D"_blank" href=3D"mailto:nghialm78@gm= ail.com?subject=3DPrivate:%20Re:%20Re%3A%20%5Blinux-safety%5D%20%5BPATCH%5D= %20parse-sublevel-option.h%20%3A%20Add%20new%20line%20and%20license%20ident= ifier">Reply To Sender</a> | <a target=3D"_blank" href=3D"mailto:linux-saf= [email protected]?subject=3DRe:%20Re%3A%20%5Blinux-safety%5D%20%5BPATCH%= 5D%20parse-sublevel-option.h%20%3A%20Add%20new%20line%20and%20license%20ide= ntifier">Reply To Group</a> | <a target=3D"_blank" href=3D"https://lists= .elisa.tech/mt/84977020/5278000">Mute This Topic</a> | <a href=3D"https://= lists.elisa.tech/g/linux-safety/post">New Topic</a><br> <a href=3D"https= ://lists.elisa.tech/g/linux-safety/editsub/5278000">Your Subscription</a> |= <a href=3D"mailto:[email protected]">Contact Group Owner= </a> | <a href=3D"https://lists.elisa.tech/g/linux-safety/unsub">Unsubscri= be</a> [[email protected]]<br> <div width=3D"1" style=3D"co= lor:white;clear:both">_._,_._,_</div>=20 --0000000000003fda5405c9e6a3d4--