[Cocci] Bad formatting when changing function prototypes

[Cocci] Bad formatting when changing function prototypes

[Joe Hershberger]

Hi,

I'm trying to use spatch to convert from one API to another across
many files and I'm running into a problem.

I don't know if I'm not configuring a setting properly or if it might be a bug.

Here's what I'm seeing... function prototypes or definitions that are
near or over the 80 char limit end up with poor formatting.

https://patchwork.ozlabs.org/patch/605756/

Eg:

-static int bfin_miiphy_read(const char *devname, uchar addr, uchar
reg, ushort *val)
+static int bfin_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg
+       )

-static int bfin_miiphy_write(const char *devname, uchar addr, uchar
reg, ushort val)
+static int bfin_miiphy_write(struct mii_dev *bus, int addr, int devad,
+                            int reg, u16 val
+       )

-static int davinci_mii_phy_read(const char *devname, unsigned char
addr, unsigned char reg, unsigned short *value)
+static int davinci_mii_phy_read(struct mii_dev *bus, int addr, int devad,
+       int reg
+       )

The worst part is that it moves the close paren to the next line.

Also, I'm not sure how it decides how much indentation to use if it
truly needs an extra line for parameters. Ideally the indentation of
the next line of parameters would match the open paren position. It
seems to do that in some cases and not in other cases. I'm not sure
what affects it.

I had started back when 1.0.4 was the most recent, and I just now
tested with both 1.0.5 and master. All behave identically.

Does anyone have an idea what I can do to affect this behavior or does
it require a code change?

Thanks,
-Joe

[Cocci] Bad formatting when changing function prototypes

[Julia Lawall]

On Fri, 5 Aug 2016, Joe Hershberger wrote:

> Hi,
>
> I'm trying to use spatch to convert from one API to another across
> many files and I'm running into a problem.
>
> I don't know if I'm not configuring a setting properly or if it might be a bug.
>
> Here's what I'm seeing... function prototypes or definitions that are
> near or over the 80 char limit end up with poor formatting.
>
> https://patchwork.ozlabs.org/patch/605756/
>
> Eg:
>
> -static int bfin_miiphy_read(const char *devname, uchar addr, uchar
> reg, ushort *val)
> +static int bfin_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg
> +       )
>
> -static int bfin_miiphy_write(const char *devname, uchar addr, uchar
> reg, ushort val)
> +static int bfin_miiphy_write(struct mii_dev *bus, int addr, int devad,
> +                            int reg, u16 val
> +       )
>
> -static int davinci_mii_phy_read(const char *devname, unsigned char
> addr, unsigned char reg, unsigned short *value)
> +static int davinci_mii_phy_read(struct mii_dev *bus, int addr, int devad,
> +       int reg
> +       )
>
> The worst part is that it moves the close paren to the next line.
>
> Also, I'm not sure how it decides how much indentation to use if it
> truly needs an extra line for parameters. Ideally the indentation of
> the next line of parameters would match the open paren position. It
> seems to do that in some cases and not in other cases. I'm not sure
> what affects it.
>
> I had started back when 1.0.4 was the most recent, and I just now
> tested with both 1.0.5 and master. All behave identically.
>
> Does anyone have an idea what I can do to affect this behavior or does
> it require a code change?

Could you send a semantic patch that causes the problem?

For function calls, there is an effort to stay within 80 characters, and
to put the extra lines just after the (.  I would have to look into why it
is not doing this for function declarations.  I don't think there is
anything you can do on your side.

julia

[Cocci] Bad formatting when changing function prototypes

[Joe Hershberger]

Hi Julia,

On Fri, Aug 5, 2016 at 3:52 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Fri, 5 Aug 2016, Joe Hershberger wrote:
>
>> Hi,
>>
>> I'm trying to use spatch to convert from one API to another across
>> many files and I'm running into a problem.
>>
>> I don't know if I'm not configuring a setting properly or if it might be a bug.
>>
>> Here's what I'm seeing... function prototypes or definitions that are
>> near or over the 80 char limit end up with poor formatting.
>>
>> https://patchwork.ozlabs.org/patch/605756/
>>
>> Eg:
>>
>> -static int bfin_miiphy_read(const char *devname, uchar addr, uchar
>> reg, ushort *val)
>> +static int bfin_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg
>> +       )
>>
>> -static int bfin_miiphy_write(const char *devname, uchar addr, uchar
>> reg, ushort val)
>> +static int bfin_miiphy_write(struct mii_dev *bus, int addr, int devad,
>> +                            int reg, u16 val
>> +       )
>>
>> -static int davinci_mii_phy_read(const char *devname, unsigned char
>> addr, unsigned char reg, unsigned short *value)
>> +static int davinci_mii_phy_read(struct mii_dev *bus, int addr, int devad,
>> +       int reg
>> +       )
>>
>> The worst part is that it moves the close paren to the next line.
>>
>> Also, I'm not sure how it decides how much indentation to use if it
>> truly needs an extra line for parameters. Ideally the indentation of
>> the next line of parameters would match the open paren position. It
>> seems to do that in some cases and not in other cases. I'm not sure
>> what affects it.
>>
>> I had started back when 1.0.4 was the most recent, and I just now
>> tested with both 1.0.5 and master. All behave identically.
>>
>> Does anyone have an idea what I can do to affect this behavior or does
>> it require a code change?
>
> Could you send a semantic patch that causes the problem?

Here is the patch that I'm using.  https://patchwork.ozlabs.org/patch/605758/

It is intended to apply to http://git.denx.de/?p=u-boot.git;a=summary

I've been executing spatch using this command line within the U-Boot
source tree:

spatch --in-place --sp-file scripts/coccinelle/net/mdio_register.cocci .

> For function calls, there is an effort to stay within 80 characters, and
> to put the extra lines just after the (.  I would have to look into why it
> is not doing this for function declarations.  I don't think there is
> anything you can do on your side.

OK, good to know. If there is anything I can do to make it easier to
reproduce, please let me know.

Thanks,
-Joe

[Cocci] Bad formatting when changing function prototypes

[Julia Lawall]

On Fri, 5 Aug 2016, Joe Hershberger wrote:

> Hi Julia,
>
> On Fri, Aug 5, 2016 at 3:52 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Fri, 5 Aug 2016, Joe Hershberger wrote:
> >
> >> Hi,
> >>
> >> I'm trying to use spatch to convert from one API to another across
> >> many files and I'm running into a problem.
> >>
> >> I don't know if I'm not configuring a setting properly or if it might be a bug.
> >>
> >> Here's what I'm seeing... function prototypes or definitions that are
> >> near or over the 80 char limit end up with poor formatting.
> >>
> >> https://patchwork.ozlabs.org/patch/605756/
> >>
> >> Eg:
> >>
> >> -static int bfin_miiphy_read(const char *devname, uchar addr, uchar
> >> reg, ushort *val)
> >> +static int bfin_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg
> >> +       )
> >>
> >> -static int bfin_miiphy_write(const char *devname, uchar addr, uchar
> >> reg, ushort val)
> >> +static int bfin_miiphy_write(struct mii_dev *bus, int addr, int devad,
> >> +                            int reg, u16 val
> >> +       )
> >>
> >> -static int davinci_mii_phy_read(const char *devname, unsigned char
> >> addr, unsigned char reg, unsigned short *value)
> >> +static int davinci_mii_phy_read(struct mii_dev *bus, int addr, int devad,
> >> +       int reg
> >> +       )
> >>
> >> The worst part is that it moves the close paren to the next line.
> >>
> >> Also, I'm not sure how it decides how much indentation to use if it
> >> truly needs an extra line for parameters. Ideally the indentation of
> >> the next line of parameters would match the open paren position. It
> >> seems to do that in some cases and not in other cases. I'm not sure
> >> what affects it.
> >>
> >> I had started back when 1.0.4 was the most recent, and I just now
> >> tested with both 1.0.5 and master. All behave identically.
> >>
> >> Does anyone have an idea what I can do to affect this behavior or does
> >> it require a code change?
> >
> > Could you send a semantic patch that causes the problem?
>
> Here is the patch that I'm using.  https://patchwork.ozlabs.org/patch/605758/
>
> It is intended to apply to http://git.denx.de/?p=u-boot.git;a=summary
>
> I've been executing spatch using this command line within the U-Boot
> source tree:
>
> spatch --in-place --sp-file scripts/coccinelle/net/mdio_register.cocci .
>
> > For function calls, there is an effort to stay within 80 characters, and
> > to put the extra lines just after the (.  I would have to look into why it
> > is not doing this for function declarations.  I don't think there is
> > anything you can do on your side.
>
> OK, good to know. If there is anything I can do to make it easier to
> reproduce, please let me know.

I have the impression that if you don't put a newline in front of the
added ) then Coccinelle also does not put one.  I don't yet know why it
behaves this way, though.

julia

[Cocci] Bad formatting when changing function prototypes

[Julia Lawall]

The problem should be fixed.  It's a little bit curious that you put
newlines in the semantic patch that you don't want to see in the code.  In
some cases, Coccinelle takes those newlines as hints for how to format the
resulting code.  But now for parameter lists (and argument lists) it will
never put a newline before the final ).

The fix is available in github.  If you have any problem in compiling,
with respect to python, do make distclean; ./autogen; ./configure before
doing make.

Thanks for the report.

julia

[Cocci] Bad formatting when changing function prototypes

[Joe Hershberger]

Hi Julia,

On Sat, Aug 6, 2016 at 5:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> The problem should be fixed.  It's a little bit curious that you put
> newlines in the semantic patch that you don't want to see in the code.  In
> some cases, Coccinelle takes those newlines as hints for how to format the
> resulting code.

Ah, I didn't realize that there was any connection. I probably would
have been more likely to make that connection if all variables were on
separate lines (like my patch - done only for readability). That's
good to know.

> But now for parameter lists (and argument lists) it will
> never put a newline before the final ).

Great, thanks.

> The fix is available in github.  If you have any problem in compiling,
> with respect to python, do make distclean; ./autogen; ./configure before
> doing make.

Building worked fine, but the autotools errantly decided to built for
PCRE support even though I didn't have it installed. I think that's
new broken behavior since the 1.0.5 release

> Thanks for the report.

Thanks for fixing it!

Cheers,
-Joe

[Cocci] Bad formatting when changing function prototypes

[Julia Lawall]

On Mon, 8 Aug 2016, Joe Hershberger wrote:

> Hi Julia,
>
> On Sat, Aug 6, 2016 at 5:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > The problem should be fixed.  It's a little bit curious that you put
> > newlines in the semantic patch that you don't want to see in the code.  In
> > some cases, Coccinelle takes those newlines as hints for how to format the
> > resulting code.
>
> Ah, I didn't realize that there was any connection. I probably would
> have been more likely to make that connection if all variables were on
> separate lines (like my patch - done only for readability). That's
> good to know.
>
> > But now for parameter lists (and argument lists) it will
> > never put a newline before the final ).
>
> Great, thanks.
>
> > The fix is available in github.  If you have any problem in compiling,
> > with respect to python, do make distclean; ./autogen; ./configure before
> > doing make.
>
> Building worked fine, but the autotools errantly decided to built for
> PCRE support even though I didn't have it installed. I think that's
> new broken behavior since the 1.0.5 release

Thanks for the report.  We will look into it.

julia

[Cocci] Bad formatting when changing function prototypes

[Thierry Martinez]

Joe Hershberger:
> Building worked fine, but the autotools errantly decided to built for
> PCRE support even though I didn't have it installed. I think that's
> new broken behavior since the 1.0.5 release

This should be fixed by the commit 99f1de0 on GitHub.

Thanks for the report.
-- 
Thierry.

Joe Hershberger (2016/08/08, 17:53 CEST)?:
> Hi Julia,
>
> On Sat, Aug 6, 2016 at 5:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> The problem should be fixed.  It's a little bit curious that you put
>> newlines in the semantic patch that you don't want to see in the code.  In
>> some cases, Coccinelle takes those newlines as hints for how to format the
>> resulting code.
>
> Ah, I didn't realize that there was any connection. I probably would
> have been more likely to make that connection if all variables were on
> separate lines (like my patch - done only for readability). That's
> good to know.
>
>> But now for parameter lists (and argument lists) it will
>> never put a newline before the final ).
>
> Great, thanks.
>
>> The fix is available in github.  If you have any problem in compiling,
>> with respect to python, do make distclean; ./autogen; ./configure before
>> doing make.
>
> Building worked fine, but the autotools errantly decided to built for
> PCRE support even though I didn't have it installed. I think that's
> new broken behavior since the 1.0.5 release
>
>> Thanks for the report.
>
> Thanks for fixing it!
>
> Cheers,
> -Joe
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Bad formatting when changing function prototypes

[Joe Hershberger]

On Tue, Aug 9, 2016 at 8:32 AM, Thierry Martinez
<Thierry.Martinez@inria.fr> wrote:
> Joe Hershberger:
>> Building worked fine, but the autotools errantly decided to built for
>> PCRE support even though I didn't have it installed. I think that's
>> new broken behavior since the 1.0.5 release
>
> This should be fixed by the commit 99f1de0 on GitHub.

Thanks!

-Joe