[PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[John Levon]

Signed-off-by: John Levon <levon@movementarian.org>
---
 lib.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib.c b/lib.c
index f20e0885..d7cd8cbc 100644
--- a/lib.c
+++ b/lib.c
@@ -325,15 +325,19 @@ static char **handle_switch_D(char *arg, char **next)
 	const char *name = arg + 1;
 	const char *value = "1";
 
-	if (!*name || isspace((unsigned char)*name))
-		die("argument to `-D' is missing");
+	if (!*name) {
+		arg = *++next;
+		if (!arg)
+			die("argument to `-D' is missing");
+		name = arg;
+	}
 
-	for (;;) {
+	for (;;arg++) {
 		char c;
-		c = *++arg;
+		c = *arg;
 		if (!c)
 			break;
-		if (isspace((unsigned char)c) || c == '=') {
+		if (c == '=') {
 			*arg = '\0';
 			value = arg + 1;
 			break;
@@ -685,6 +689,8 @@ static void handle_switch_v_finalize(void)
 static char **handle_switch_U(char *arg, char **next)
 {
 	const char *name = arg + 1;
+	if (*name == '\0')
+		name = *++next;
 	add_pre_buffer ("#undef %s\n", name);
 	return next;
 }
-- 
2.14.1

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[Luc Van Oostenryck]

On Thu, Nov 22, 2018 at 12:08:06PM +0000, John Levon wrote:
> Signed-off-by: John Levon <levon@movementarian.org>
> ---
>  lib.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib.c b/lib.c
> index f20e0885..d7cd8cbc 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -325,15 +325,19 @@ static char **handle_switch_D(char *arg, char **next)
>  	const char *name = arg + 1;
>  	const char *value = "1";
>  
> -	if (!*name || isspace((unsigned char)*name))
> -		die("argument to `-D' is missing");
> +	if (!*name) {
> +		arg = *++next;
> +		if (!arg)
> +			die("argument to `-D' is missing");
> +		name = arg;
> +	}
>  
> -	for (;;) {
> +	for (;;arg++) {
>  		char c;
> -		c = *++arg;
> +		c = *arg;
>  		if (!c)
>  			break;
> -		if (isspace((unsigned char)c) || c == '=') {
> +		if (c == '=') {
>  			*arg = '\0';
>  			value = arg + 1;
>  			break;

Hi,

This is copied from existing patches from sparse:
  https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=2f922ba822da324ccd2c201c076ca94d5e910a8a
  https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=d32b2f7c202fd27206169ca99da898ca9c20dfab
without giving credit and without respecting the Signed-off-by /
Developer Certificate of Origin (cfr. https://developercertificate.org/).
Please fix this.

-- Luc Van Oostenryck

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[John Levon]

On Sun, Nov 25, 2018 at 10:29:56PM +0100, Luc Van Oostenryck wrote:

> This is copied from existing patches from sparse:
>   https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=2f922ba822da324ccd2c201c076ca94d5e910a8a
>   https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=d32b2f7c202fd27206169ca99da898ca9c20dfab

Hi Luc, apologies. I did in fact mention this patch was in part from
upstream.

> without giving credit and without respecting the Signed-off-by /
> Developer Certificate of Origin (cfr. https://developercertificate.org/).

I'm not clear how this isn't covered under part b) here.

Please clarify exactly what you would like me to do.

regards
john

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[Luc Van Oostenryck]

On Sun, Nov 25, 2018 at 11:29:14PM +0000, John Levon wrote:
> On Sun, Nov 25, 2018 at 10:29:56PM +0100, Luc Van Oostenryck wrote:
> 
> > This is copied from existing patches from sparse:
> >   https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=2f922ba822da324ccd2c201c076ca94d5e910a8a
> >   https://git.kernel.org/pub/scm/devel/sparse/sparse.git/commit/?id=d32b2f7c202fd27206169ca99da898ca9c20dfab
> 
> Hi Luc, apologies. I did in fact mention this patch was in part from
> upstream.

No problem.
Sorry, but I don't see this in the patch/email itself (and I didn't see
a "cover letter" explaining the whole series).

> > without giving credit and without respecting the Signed-off-by /
> > Developer Certificate of Origin (cfr. https://developercertificate.org/).
> 
> I'm not clear how this isn't covered under part b) here.
> 
> Please clarify exactly what you would like me to do.

Hi,

The signed-off-by should be like:
	Signed-off-by: Original Author <author@example.com>
	Signed-off-by: John Levon <levon@movementarian.org>
That's for the DCO (the idea is to be able to somehow track the
origin of the patch and who has possibly modified it). The GIT_AUTHOR
should also be preserved. When resending a patch, this can be done
by adding in the very first line of the message body a line like:
	From: Original Author <author@example.com>
(if the patch is then taken via git-am and some other tools).

Putting this DCO question aside, I would find normal that the commit
message would contains a small note adding something like:
	[This patch was originally written by ...]

Now, I also think that these patches, touching the sparse's part of
smatch, will make things more difficult when/if smatch's tree will
be synchronized with sparse's. Even more so if they are slightly
different than sparse's upstream. Wouldn't it be better to:
1) ask Dan (smatch's maintainer) if he could take this and this
   patches in sparse's upstream because they solve this and this
   problem for you;
2) also submit the ones you wrote yourself to sparse's mailing list
   so that both smatch and sparse can benefit from the improvement?

-- Luc Van Oostenryck

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[Dan Carpenter]

It looks like everything was BCC instead of To: and Cc:?  I can't tell
which went to linux-sparse and which smatch@vger.kernel.org.  Smatch is
GPL and Sparse is MIT, but any shared code is MIT licensed.

I would like to sync with upstream Sparse, but I tried a couple months
back and it got complicated so I have procrastinated.  But I could just
cherry-pick those two patches.  Or John, you could cherry-pick them and
send them to me.  `man git cherry-pick`.

regards,
dan carpenter

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[John Levon]

On Mon, Nov 26, 2018 at 01:27:35AM +0100, Luc Van Oostenryck wrote:

> Putting this DCO question aside, I would find normal that the commit
> message would contains a small note adding something like:
>       [This patch was originally written by ...]

It's in the subject (where it ended up from git format-patch). Dan has
already taken me to task for the format of the commits here.

> The signed-off-by should be like:
>       Signed-off-by: Original Author <author@example.com>
>       Signed-off-by: John Levon <levon@movementarian.org>

This sounds awfully like *you've* signed off on *this* patch, but sure,
whatever is the usual way.

On Mon, Nov 26, 2018 at 10:49:09AM +0300, Dan Carpenter wrote:

> It looks like everything was BCC instead of To: and Cc:?  I can't tell
> which went to linux-sparse and which smatch@vger.kernel.org.  Smatch is
> GPL and Sparse is MIT, but any shared code is MIT licensed.

I screwed up the mailing (next ones will be better), but they all went
to smatch. As being in smatch and not sparse is of no use to us, I
thought this made sense right now at least until smatch is nearer
upstream.

> cherry-pick those two patches.  Or John, you could cherry-pick them and
> send them to me.  `man git cherry-pick`.

There's a good few more than just two. If you'd prefer, I can work on
taking them upstream first?

regards
john

Re: [PATCH 05/18] sparse: correctly handle "-D foo" and "-U foo". The former is from sparse upstream, but they didn't fix the latter for some reason.

[Dan Carpenter]

On Mon, Nov 26, 2018 at 08:44:36AM +0000, John Levon wrote:
> On Mon, Nov 26, 2018 at 01:27:35AM +0100, Luc Van Oostenryck wrote:
> 
> > Putting this DCO question aside, I would find normal that the commit
> > message would contains a small note adding something like:
> >       [This patch was originally written by ...]
> 
> It's in the subject (where it ended up from git format-patch). Dan has
> already taken me to task for the format of the commits here.
> 
> > The signed-off-by should be like:
> >       Signed-off-by: Original Author <author@example.com>
> >       Signed-off-by: John Levon <levon@movementarian.org>
> 
> This sounds awfully like *you've* signed off on *this* patch, but sure,
> whatever is the usual way.
>

The logic there is that everyone who touches the patch has to sign off
that they didn't add any Secret SCO Unix Source Code to the patch.

> On Mon, Nov 26, 2018 at 10:49:09AM +0300, Dan Carpenter wrote:
> 
> > It looks like everything was BCC instead of To: and Cc:?  I can't tell
> > which went to linux-sparse and which smatch@vger.kernel.org.  Smatch is
> > GPL and Sparse is MIT, but any shared code is MIT licensed.
> 
> I screwed up the mailing (next ones will be better), but they all went
> to smatch. As being in smatch and not sparse is of no use to us, I
> thought this made sense right now at least until smatch is nearer
> upstream.
> 
> > cherry-pick those two patches.  Or John, you could cherry-pick them and
> > send them to me.  `man git cherry-pick`.
> 
> There's a good few more than just two. If you'd prefer, I can work on
> taking them upstream first?

Upstreaming first is more ideal, but I'll take them as-is if you want.
Smatch licensing allows anyone to upstream shared code from Smatch to
Sparse after the fact as well.

regards,
dan carpenter

Sync smatch with sparse?

[Luc Van Oostenryck]

On Mon, Nov 26, 2018 at 10:49:09AM +0300, Dan Carpenter wrote:
> It looks like everything was BCC instead of To: and Cc:?  I can't tell
> which went to linux-sparse and which smatch@vger.kernel.org.  Smatch is
> GPL and Sparse is MIT, but any shared code is MIT licensed.
> 
> I would like to sync with upstream Sparse, but I tried a couple months
> back and it got complicated so I have procrastinated.

Hi Dan,

It would indeed be good to sync smatch & sparse and I would be ready,
if needed, to make some changes to sparse to make it easier.
There is also several ways to do that. I'm wondering if you
have already considered to simply remove anything related to
sparse from smatch's tree and install sparse as a seperate library?

Another approach would be to use consider sparse as a subtree
(in git-subtree's sense) of smatch (in a directory 'sparse').
If you're interested in this approach, I've put a sort of draft
of it in:
	git://github.com/lucvoo/smatch.git split-sparse

Currently, you first need to:
	make -C sparse
in this tree since sprase is there in an independent subdir
but that would be easy enough to call from the main Makefile
(which is now much simpler).

Best regards,
-- Luc

Re: Sync smatch with sparse?

[John Levon]

On Thu, Nov 29, 2018 at 11:03:28am +0100, Luc Van Oostenryck wrote:

> On Mon, Nov 26, 2018 at 10:49:09AM +0300, Dan Carpenter wrote:
> > It looks like everything was BCC instead of To: and Cc:?  I can't tell
> > which went to linux-sparse and which smatch@vger.kernel.org.  Smatch is
> > GPL and Sparse is MIT, but any shared code is MIT licensed.
> > 
> > I would like to sync with upstream Sparse, but I tried a couple months
> > back and it got complicated so I have procrastinated.
> 
> Hi Dan,
> 
> It would indeed be good to sync smatch & sparse and I would be ready,
> if needed, to make some changes to sparse to make it easier.
> There is also several ways to do that. I'm wondering if you
> have already considered to simply remove anything related to
> sparse from smatch's tree and install sparse as a seperate library?

Right now I at least have changes in sparse that are very much
smatch-dependent, so this would be a real pain for me...

regards
john