update to format parsing branch

update to format parsing branch

[Ben Dooks]

I've done a rebase to v0.6.2 and put the result up at:

https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3

Should I put this through a new round of review?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: update to format parsing branch

[Luc Van Oostenryck]

On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
> I've done a rebase to v0.6.2 and put the result up at:
> 
> https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3

Great.
It just needs a few very mnor changes:

%% diff --git a/evaluate.h b/evaluate.h
%% index a16e97036b2a..bb8ec480691c 100644
%% --- a/evaluate.h
%% +++ b/evaluate.h
%% @@ -26,10 +26,14 @@ struct symbol *evaluate_statement(struct statement *stmt);
%%  // @list: the list of the symbol to be evaluated
%%  void evaluate_symbol_list(struct symbol_list *list);
%%  
%% -///

This is the marker for autodoc, so it needs to stay.

%%  // evaluate the arguments of a function
%% +// @fn: the symbol of the prototype
%%  // @argtypes: the list of the types in the prototype
%%  // @args: the list of the effective arguments
%% -int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *args);
%% +int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes, struct expression_list *args);
%%  
%% +// check if assignment types are compatible
%% +// todo

I've removed the 'todo'

%% diff --git a/parse.c b/parse.c
%% index 31ecef0f554d..a7ab5fd6e531 100644
%% --- a/parse.c
%% +++ b/parse.c
%% @@ -377,6 +383,10 @@ static struct symbol_op attr_force_op = {
%%  	.attribute = attribute_force,
%%  };
%%  
%% +static struct symbol_op attr_format = {

To stay coherent with existent naming, I've rename this into 'attr_format_op'

%% @@ -515,6 +535,7 @@ static struct init_keyword {
%%  	N("_Float64",		&spec_op,	.type = &float64_ctype),
%%  	N("_Float64x",		&spec_op,	.type = &float64x_ctype),
%%  	N("_Float128",		&spec_op,	.type = &float128_ctype),
%% +

Removd unneeded line.

%% @@ -551,6 +572,9 @@ static struct init_keyword {
%%  	D("pure",		&attr_fun_op,		.mods = MOD_PURE),
%%  	A("const",		&attr_fun_op,		.mods = MOD_PURE),
%%  	D("gnu_inline",		&attr_fun_op,		.mods = MOD_GNU_INLINE),
%% +	N("format",		&attr_format),
%% +	N("printf",		&attr_printf_op),
%% +	N("scanf",		&attr_scanf_op),

I've changed the 'N' into 'D' since the underscore versions are legit and
often favored.

%% diff --git a/sparse.1 b/sparse.1
%% index 48dab7a9a5c1..e46aafdb3e5e 100644
%% --- a/sparse.1
%% +++ b/sparse.1
%% @@ -275,6 +275,15 @@ trouble.
%%  Sparse does not issue these warnings by default.
%%  .
%%  .TP
%% +.B \-Wformat
%% +Warn about parameter mismatch to any variadic function which specifies
%% +where the format string is specified with the 
%% +.BI __attribute__((format( type, message, va_start )))
%% +attribute.
%% +
%% +Sparse does not issue these warnings by default. To turn them on, use
%% +\fB\-Wno\-format\fR
%% +.TP

I've moved this one position above, between '-Wexternal-...' and -Winit-..
to keep the alphabetical order, removed some unneeded whitespace and
added a final dot.

%% diff --git a/symbol.h b/symbol.h
%% index a3ed95678ee5..47e26816430c 100644
%% --- a/symbol.h
%% +++ b/symbol.h
%% @@ -84,6 +84,7 @@ enum keyword {
%%  	KW_STATIC	= 1 << 7,
%%       // KW UNUSED	= 1 << 8,
%%  	KW_EXACT	= 1 << 9,
%% +	KW_FORMAT	= 1 << 10,
%%  };

I've just reused the UNUSED slot.


I've also exchanged patch 3 & 4 because the 3rd needed the definition of
Wformat only added in patch 4 and fixed 2 or 3 typos in the commit messages. 
I've pushed these changes at gitlab.com/lucvoo/sparse-dev format-check

I've left for now the changelog parts in the commit messages but I would
prefer to not have them in the final version.

> Should I put this through a new round of review?

I'm fine with the changes (moving the check to a verify-format.c +
some changes related to excessively long lines).

I've one last request, the email address given as author is not the same
as the one used in the Signed-off-by and the copyright notice.

So, is it possible to respin the series with:
* the small changes I've made here above
* removing the changelogs from the commit messages
* using the same email address for the author and the SoB?

Thanks,
-- Luc

Re: update to format parsing branch

[Ben Dooks]

On 30/09/2020 21:29, Luc Van Oostenryck wrote:
> On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
>> I've done a rebase to v0.6.2 and put the result up at:
>>
>> https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3

[snip]

> So, is it possible to respin the series with:
> * the small changes I've made here above
> * removing the changelogs from the commit messages
> * using the same email address for the author and the SoB?

Ok, will sort these out tonight.

What's the best branch to base-on for merge?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: update to format parsing branch

[Luc Van Oostenryck]

On Wed, Sep 30, 2020 at 09:59:12PM +0100, Ben Dooks wrote:
> On 30/09/2020 21:29, Luc Van Oostenryck wrote:
> > On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
> > > I've done a rebase to v0.6.2 and put the result up at:
> > > 
> > > https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3
> 
> [snip]
> 
> > So, is it possible to respin the series with:
> > * the small changes I've made here above
> > * removing the changelogs from the commit messages
> > * using the same email address for the author and the SoB?
> 
> Ok, will sort these out tonight.
> 
> What's the best branch to base-on for merge?

The one you used for your printf-new3:
	24bdaac6682c ("Merge branch 'linear-fma' into next")

is perfect.
-- Luc

Re: update to format parsing branch

[Ben Dooks]

On 30/09/2020 21:29, Luc Van Oostenryck wrote:
> On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
>> I've done a rebase to v0.6.2 and put the result up at:
>>
>> https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3
> 
> Great.
> 
> I've one last request, the email address given as author is not the same
> as the one used in the Signed-off-by and the copyright notice.
> 
> So, is it possible to respin the series with:
> * the small changes I've made here above
> * removing the changelogs from the commit messages
> * using the same email address for the author and the SoB?

I can't see any diffs between git-authour and the SoB in the series.



-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: update to format parsing branch

[Luc Van Oostenryck]

On Wed, Sep 30, 2020 at 10:05:36PM +0100, Ben Dooks wrote:
> On 30/09/2020 21:29, Luc Van Oostenryck wrote:
> > On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
> > > I've done a rebase to v0.6.2 and put the result up at:
> > > 
> > > https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3
> > 
> > Great.
> > 
> > I've one last request, the email address given as author is not the same
> > as the one used in the Signed-off-by and the copyright notice.
> > 
> > So, is it possible to respin the series with:
> > * the small changes I've made here above
> > * removing the changelogs from the commit messages
> > * using the same email address for the author and the SoB?
> 
> I can't see any diffs between git-authour and the SoB in the series.

	$ git log 5c15b086cd50
	commit 5c15b086cd501633be2ad5fedeeb97c09874409a (bjdooks/bjdooks/printf-new3)
	Author: Ben Dooks <ben-linux@fluff.org>
	Date:   2018-10-30 10:36:26 +0000
	
	    add -Wformat
	    
	    Add option to enable/disable format checking (and default it to off)
	    
	    Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>


The author is:  Ben Dooks <ben-linux@fluff.org>
but the SoB is: Ben Dooks <ben.dooks@codethink.co.uk>

-- Luc

Re: update to format parsing branch

[Ben Dooks]

On 30/09/2020 22:39, Luc Van Oostenryck wrote:
> On Wed, Sep 30, 2020 at 10:05:36PM +0100, Ben Dooks wrote:
>> On 30/09/2020 21:29, Luc Van Oostenryck wrote:
>>> On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote:
>>>> I've done a rebase to v0.6.2 and put the result up at:
>>>>
>>>> https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3
>>>
>>> Great.
>>>
>>> I've one last request, the email address given as author is not the same
>>> as the one used in the Signed-off-by and the copyright notice.
>>>
>>> So, is it possible to respin the series with:
>>> * the small changes I've made here above
>>> * removing the changelogs from the commit messages
>>> * using the same email address for the author and the SoB?
>>
>> I can't see any diffs between git-authour and the SoB in the series.
> 
> 	$ git log 5c15b086cd50
> 	commit 5c15b086cd501633be2ad5fedeeb97c09874409a (bjdooks/bjdooks/printf-new3)
> 	Author: Ben Dooks <ben-linux@fluff.org>
> 	Date:   2018-10-30 10:36:26 +0000
> 	
> 	    add -Wformat
> 	
> 	    Add option to enable/disable format checking (and default it to off)
> 	
> 	    Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

Ah, looks like my rebasing has re-authoured it.



-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html