Re: [squid-dev] NOTICE: astyle brokenness
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 20/12/2014 9:44 p.m., Amos Jeffries wrote: With the hint of Markus recent patch containing tab indentation, I have looked at why our sourcemaintenance script did not already clean it up. It turns out there was a triplet of bugs which on the new Ubuntu server are combining to mean astyle 2.03 made almost none of the changes which it should have been for the last several months. (ouch!) Updating the astyle version to 2.04 plus bug fixes to our scripts resolves all these issues. It also resolves some very old stylistic things we have been working around in the 1.23 version for years. Please be prepared for a large speed-bump patch from source maintenance on the next enforcement scan. I am not sure what rev. number we will be up to at the time, I'll call it $N for now. There we go. It has happened. On 21/12/2014 1:14 a.m., Cron Daemon wrote: Committed revision 13776. So: You can update branches incrementally like so to avoid conflict worries: bzr merge -r 13775 # fix any merge conflicts as normal bzr commit -m Merged from trunk rev.$N-1 bzr merge -c 13776 bzr resolve --take-this # run ./scripts/sourcemaintenance.sh yourself bzr commit Then possibly bzr merge again for importing further trunk changes. Amos -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJUlYFAAAoJELJo5wb/XPRjiTMIANHjJwNZF+VIMgayKxBtQQv8 FAZIg/0G/qgjVoFg/RF/K9BzNyKnmO8EpXaFx5DACK9fRTPOx6H0RMgymlTzRgJu PPbxWS7+aaX4t1ngu7dwNpFHjO6gYlp3wXj5yohQv+5JHBsek1aTqOi4EfTPd7dV q5qglzY4Z9G/lNXVgWok25Ct2gF8mMPK2QKWA0/nQFBWjS27x/6Est1SAp3TQ37k oKlK1FipFB99HvBZnkpPKYIoDGpJRN0+Jq1w98BBCA36d73sk0m8JXPkN0X/rzvQ Xcd6W3R3hBji3JITO53mU8XNyMyuJq5q4hwvAS4lSJbna+BD5aMlBwswGkCwNr0= =SlPu -END PGP SIGNATURE- ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: Astyle version?
On 08/12/10 19:47, Amos Jeffries wrote: On 08/12/10 19:32, Amos Jeffries wrote: On 08/12/10 18:45, Amos Jeffries wrote: On 08/12/10 13:35, Henrik Nordström wrote: tis 2010-12-07 klockan 11:06 +0100 skrev Henrik Nordström: tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. east is now ready for testing astyle etc. Current status is FreeBSD-7.3-P4 with fully up to date ports. key items: astyle-1.24 automake-1.11.1 autoconf-2.68 libtool-2.4 bzr-2.2.2 + bzrtools-2.2.0 Regards Henrik pico (or the improved fork 'nano') seems to be missing on east. Updating the script via other means and it cant find md5sum. ah, NX that last. the script was not looking for the east hostname. 'md5' is fine. The outlook is both good and terrible: It seems to be sucking the life out of 3 CPU and 1GB of RAM and still only inching its way through the files. In the same time it takes 1.23 to complete the full sources 1.24 has almost finished the compat/ folder. File-wise so far the files have turned out either unchanged or truncated. MD5 catches and prevents that last from breaking the code. Found out why the truncation happens. It dies after sucking up 3.5GB of RAM on some files. Amos ... 5 hrs later its done compat/, lib/, include/ and helpers. Got a short way into the src/ directory top level. It has cleaned up a few weirdnesses from 1.23. The results of the actual formating look better so far. The speed is a blocker bug though. Amos -- Please be using Current Stable Squid 2.7.STABLE9 or 3.1.9 Beta testers wanted for 3.2.0.3
Re: Astyle version?
ons 2010-12-08 klockan 18:45 +1300 skrev Amos Jeffries: pico (or the improved fork 'nano') seems to be missing on east. nano now installed. There is also ee, vi, vim, edit and some more.. Regards Henrik
Re: Astyle version?
On Wed, 08 Dec 2010 19:05:08 +0100, Henrik Nordström wrote: ons 2010-12-08 klockan 18:45 +1300 skrev Amos Jeffries: pico (or the improved fork 'nano') seems to be missing on east. nano now installed. There is also ee, vi, vim, edit and some more.. Regards Henrik Thanks Henrik. Amos
Re: Astyle version?
tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. Regards Henrik
Re: Astyle version?
tis 2010-12-07 klockan 11:06 +0100 skrev Henrik Nordström: tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. east is now ready for testing astyle etc. Current status is FreeBSD-7.3-P4 with fully up to date ports. key items: astyle-1.24 automake-1.11.1 autoconf-2.68 libtool-2.4 bzr-2.2.2 + bzrtools-2.2.0 Regards Henrik
Re: Astyle version?
On 08/12/10 13:35, Henrik Nordström wrote: tis 2010-12-07 klockan 11:06 +0100 skrev Henrik Nordström: tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. east is now ready for testing astyle etc. Current status is FreeBSD-7.3-P4 with fully up to date ports. key items: astyle-1.24 automake-1.11.1 autoconf-2.68 libtool-2.4 bzr-2.2.2 + bzrtools-2.2.0 Regards Henrik pico (or the improved fork 'nano') seems to be missing on east. Updating the script via other means and it cant find md5sum. Amos -- Please be using Current Stable Squid 2.7.STABLE9 or 3.1.9 Beta testers wanted for 3.2.0.3
Re: Astyle version?
On 08/12/10 18:45, Amos Jeffries wrote: On 08/12/10 13:35, Henrik Nordström wrote: tis 2010-12-07 klockan 11:06 +0100 skrev Henrik Nordström: tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. east is now ready for testing astyle etc. Current status is FreeBSD-7.3-P4 with fully up to date ports. key items: astyle-1.24 automake-1.11.1 autoconf-2.68 libtool-2.4 bzr-2.2.2 + bzrtools-2.2.0 Regards Henrik pico (or the improved fork 'nano') seems to be missing on east. Updating the script via other means and it cant find md5sum. ah, NX that last. the script was not looking for the east hostname. 'md5' is fine. The outlook is both good and terrible: It seems to be sucking the life out of 3 CPU and 1GB of RAM and still only inching its way through the files. In the same time it takes 1.23 to complete the full sources 1.24 has almost finished the compat/ folder. File-wise so far the files have turned out either unchanged or truncated. MD5 catches and prevents that last from breaking the code. Amos -- Please be using Current Stable Squid 2.7.STABLE9 or 3.1.9 Beta testers wanted for 3.2.0.3
Re: Astyle version?
On 08/12/10 19:32, Amos Jeffries wrote: On 08/12/10 18:45, Amos Jeffries wrote: On 08/12/10 13:35, Henrik Nordström wrote: tis 2010-12-07 klockan 11:06 +0100 skrev Henrik Nordström: tis 2010-12-07 klockan 16:00 +1300 skrev Amos Jeffries: A test to make sure it does not add any problems would be good in advance if possible though. Sure. A full test can be run on east shortly. east is now ready for testing astyle etc. Current status is FreeBSD-7.3-P4 with fully up to date ports. key items: astyle-1.24 automake-1.11.1 autoconf-2.68 libtool-2.4 bzr-2.2.2 + bzrtools-2.2.0 Regards Henrik pico (or the improved fork 'nano') seems to be missing on east. Updating the script via other means and it cant find md5sum. ah, NX that last. the script was not looking for the east hostname. 'md5' is fine. The outlook is both good and terrible: It seems to be sucking the life out of 3 CPU and 1GB of RAM and still only inching its way through the files. In the same time it takes 1.23 to complete the full sources 1.24 has almost finished the compat/ folder. File-wise so far the files have turned out either unchanged or truncated. MD5 catches and prevents that last from breaking the code. Found out why the truncation happens. It dies after sucking up 3.5GB of RAM on some files. Amos -- Please be using Current Stable Squid 2.7.STABLE9 or 3.1.9 Beta testers wanted for 3.2.0.3
Astyle version?
Looking into upgrading squid-cache.org, and as part of this astyle will get updated to version 1.24 (1.23 installed today). Is this ok, or will it screw up the source formatting job? Regards Henrik
Re: Astyle version?
On 07/12/10 12:50, Henrik Nordström wrote: Looking into upgrading squid-cache.org, and as part of this astyle will get updated to version 1.24 (1.23 installed today). Is this ok, or will it screw up the source formatting job? Version does not matter hugely beyond the minimum of 1.22. We just need the scripts synced with the version running on the maintenance machine. To prevent flapping minor changes source-maintenance.sh will detect and skip the formater.pl for all other versions. A test to make sure it does not add any problems would be good in advance if possible though. Amos -- Please be using Current Stable Squid 2.7.STABLE9 or 3.1.9 Beta testers wanted for 3.2.0.3
Re: astyle
Tsantilas Christos wrote:
Hi Amos,
I had post the latest version of the formatter (plus a small script
for checking md5 signatures):
http://www.mail-archive.com/squid-dev@squid-cache.org/msg07208.html
In all test I run, worked without problems.
I think it is OK.
Regards,
Christos
Finally got around to running it myself on just src/*.*
For me md5checker detects bungled files. I haven't looked further than
those files to see if other directories are bunged as well.
md5checker.sh assures us the printable byte order is identical, yes?
access_log.cc - converts the prepared array of log format tokens
removing the \n placed to make the list human readable.
This is a big issue. The array entries which are properly documented
show up worst.
gopher.cc - I couldn't find it. But the diff itself could hide a lot.
Some fairly large chunks of code being shifted left by astyle and get
fully shuffled by a diff.
Of the rest all I could see was comments being shifted inside { . which
is okay.
dnsserver.cc
HttpHdrRange.cc
snmp_agent.cc
Amos
--
Please use Squid 2.7.STABLE4 or 3.0.STABLE9
Re: astyle
Amos Jeffries wrote:
Finally got around to running it myself on just src/*.*
For me md5checker detects bungled files. I haven't looked further than
those files to see if other directories are bunged as well.
md5checker.sh assures us the printable byte order is identical, yes?
This script just removes all spaces,tabs and newlines from the files
(converted and original) and computes its md5. It fails if a comment has
moved to an other position.
eg the code:
int i;/*Test comment*/
converted to:
/*Test comment*/
int i;
access_log.cc - converts the prepared array of log format tokens
removing the \n placed to make the list human readable.
This is a big issue. The array entries which are properly documented
show up worst.
yep true, if you are using astyle 1.21. I test it with astyle 1.22 too
and looks OK know :-)
gopher.cc - I couldn't find it. But the diff itself could hide a lot.
Some fairly large chunks of code being shifted left by astyle and get
fully shuffled by a diff.
Of the rest all I could see was comments being shifted inside { . which
is okay.
dnsserver.cc
HttpHdrRange.cc
snmp_agent.cc
Amos
Re: astyle
Tsantilas Christos wrote:
Amos Jeffries wrote:
Finally got around to running it myself on just src/*.*
For me md5checker detects bungled files. I haven't looked further than
those files to see if other directories are bunged as well.
md5checker.sh assures us the printable byte order is identical, yes?
This script just removes all spaces,tabs and newlines from the files
(converted and original) and computes its md5. It fails if a comment has
moved to an other position.
eg the code:
int i;/*Test comment*/
converted to:
/*Test comment*/
int i;
access_log.cc - converts the prepared array of log format tokens
removing the \n placed to make the list human readable.
This is a big issue. The array entries which are properly documented
show up worst.
yep true, if you are using astyle 1.21. I test it with astyle 1.22 too
and looks OK know :-)
Okay, in that case we need some form of version test to ensure the
installed astyle version is usable. I guess I'll have to locate a 1.22.
FWIW: I checked my astyle version against the one listed in formater.pl
to check that it was new enough. The fail still shows 1.21.
gopher.cc - I couldn't find it. But the diff itself could hide a lot.
Some fairly large chunks of code being shifted left by astyle and get
fully shuffled by a diff.
Of the rest all I could see was comments being shifted inside { .
which is okay.
dnsserver.cc
HttpHdrRange.cc
snmp_agent.cc
Amos
Amos
--
Please use Squid 2.7.STABLE4 or 3.0.STABLE9
Re: astyle
Amos Jeffries wrote:
Alex Rousskov wrote:
On Mon, 2008-01-07 at 23:55 +0200, Tsantilas Christos wrote:
Alex Rousskov wrote:
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
...
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check
whether
there is another --brackets option that works without the above bug and
does the subset of what --brackets=linux does?
We can only use the --brackets=break option which works well.
This option format the code as:
void Foo(bool isFoo)
{
if (isFoo)
{
bar();
}
else
{
anotherBar();
}
}
(This format is awful but considering the hours I spent to merge the
HEAD in async-calls ... it is the best :-) ! )
What do others think regarding making the above a temporary format
default until astyle improves or a better replacement is available?
[Not] spending hours on merging branches can be a strong-enough
motivation to use less-than-perfect format above...
Or would it be better to post-process atyle-formatted code to untangle
Less processing the better.
So putting up with a small ugly in exchange for a complicated hack is
okay by me.
On the bit-field problem, I have a similar mind. Even though the wrap is
extremely ugly the suggested fix makes code almost unreadable.
If we have to go the way of hacking bitfields around astyle, I would
suggest going to a macro (yuck). Like so:
#define BITFIELD(name,bits) unsigned int name : bits
struct {
BITFIELD(name, 1);
BITFIELD(flag, 1);
}
#if FOO {
}
lines?
Cheers,
Alex.
Amos
How was this going Christos?
We are getting close to feature finalization on 3.1.
Amos
--
Please use Squid 2.7.STABLE4 or 3.0.STABLE8
Re: astyle
Hi Amos,
I had post the latest version of the formatter (plus a small script
for checking md5 signatures):
http://www.mail-archive.com/squid-dev@squid-cache.org/msg07208.html
In all test I run, worked without problems.
I think it is OK.
Regards,
Christos
Amos Jeffries wrote:
Amos Jeffries wrote:
Alex Rousskov wrote:
On Mon, 2008-01-07 at 23:55 +0200, Tsantilas Christos wrote:
Alex Rousskov wrote:
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
...
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check
whether
there is another --brackets option that works without the above bug
and
does the subset of what --brackets=linux does?
We can only use the --brackets=break option which works well.
This option format the code as:
void Foo(bool isFoo)
{
if (isFoo)
{
bar();
}
else
{
anotherBar();
}
}
(This format is awful but considering the hours I spent to merge the
HEAD in async-calls ... it is the best :-) ! )
What do others think regarding making the above a temporary format
default until astyle improves or a better replacement is available?
[Not] spending hours on merging branches can be a strong-enough
motivation to use less-than-perfect format above...
Or would it be better to post-process atyle-formatted code to untangle
Less processing the better.
So putting up with a small ugly in exchange for a complicated hack is
okay by me.
On the bit-field problem, I have a similar mind. Even though the wrap
is extremely ugly the suggested fix makes code almost unreadable.
If we have to go the way of hacking bitfields around astyle, I would
suggest going to a macro (yuck). Like so:
#define BITFIELD(name,bits) unsigned int name : bits
struct {
BITFIELD(name, 1);
BITFIELD(flag, 1);
}
#if FOO {
}
lines?
Cheers,
Alex.
Amos
How was this going Christos?
We are getting close to feature finalization on 3.1.
Amos
Re: [MERGE] Cleanup astyle whitespace crap
On Sun, 2008-03-16 at 22:01 -0600, Alex Rousskov wrote: Should not these formatting changes wait for the global astyle+wrapper application, to minimize the number of times folks need to resolve conflicts in their branches? I think it's better to get this cleaned up soon, before there is too many active branches again (at the moment most major ones are merged).. with these cleanups resolved there should also be considerably less conflicts the day we automate styling of the tree again. I prefer smaller incremental steps improving the style, than massive change all at once. Often easier to resolve conflicts that way in a reliable manner. Regards Henrik
Re: [MERGE] Cleanup astyle whitespace crap
Bundle Buggy has detected this merge request. For details, see: http://squid-cache.org/bundlebuggy//request/%3C1205705885.12036.4.camel%40HenrikLaptop%3E
Re: [MERGE] Cleanup astyle whitespace crap
On Sun, 2008-03-16 at 22:19 +, Bundle Buggy wrote: Bundle Buggy has detected this merge request. For details, see: http://squid-cache.org/bundlebuggy//request/%3C1205705885.12036.4.camel%40HenrikLaptop%3E bb:comment Should not these formatting changes wait for the global astyle+wrapper application, to minimize the number of times folks need to resolve conflicts in their branches? Alex.
Re: astyle
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
I spend some time to run the astyle-1.21 on squid3 code. I tried to run
it with the following parameters:
--mode=c -s4 -O --break-blocks -l
I am only seeing the following two problems:
1) The --break-blocks and -l option.
Using these options the code:
#ifdef SOME
{
}
Converted to:
#ifdef SOME {
}
Maybe we can omit these options
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check whether
there is another --brackets option that works without the above bug and
does the subset of what --brackets=linux does?
2) Bit fields in structs are not formated well.
struct
{
unsigned int open:1;
unsigned int close_request:1;
unsigned int write_daemon:1;
...
}
Converted to:
struct
{
unsigned int open:
1;
unsigned int close_request:
1;
unsigned int write_daemon:
1;
...
}
This cannot be disabled, right? If all bit-fields are unsigned int,
perhaps we can run a simple pre-processor that will convert
unsigned int foo : 1;
into
unsigned int foo__FORASTYLE__1;
and then post-process the sources to undo the conversion?
Finally, please consider reporting the above bugs to astyle if nobody
has done that already.
Thank you,
Alex.
Re: astyle
Alex Rousskov wrote:
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
...
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check whether
there is another --brackets option that works without the above bug and
does the subset of what --brackets=linux does?
We can only use the --brackets=break option which works well.
This option format the code as:
void Foo(bool isFoo)
{
if (isFoo)
{
bar();
}
else
{
anotherBar();
}
}
(This format is awful but considering the hours I spent to merge the
HEAD in async-calls ... it is the best :-) ! )
2) Bit fields in structs are not formated well.
This cannot be disabled, right? If all bit-fields are unsigned int,
perhaps we can run a simple pre-processor that will convert
unsigned int foo : 1;
into
unsigned int foo__FORASTYLE__1;
and then post-process the sources to undo the conversion?
This is not bad idea. Maybe we can fix other problems too using this method.
With a (very) quick view looks that all bit fields are unsigned ints.
Finally, please consider reporting the above bugs to astyle if nobody
has done that already.
The bugs are reported. And many other too :-) . I frightened a little
looking in astyle bugzila:
http://sourceforge.net/tracker/?group_id=2319atid=102319
Re: astyle
On Mon, 2008-01-07 at 23:55 +0200, Tsantilas Christos wrote:
Alex Rousskov wrote:
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
...
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check whether
there is another --brackets option that works without the above bug and
does the subset of what --brackets=linux does?
We can only use the --brackets=break option which works well.
This option format the code as:
void Foo(bool isFoo)
{
if (isFoo)
{
bar();
}
else
{
anotherBar();
}
}
(This format is awful but considering the hours I spent to merge the
HEAD in async-calls ... it is the best :-) ! )
What do others think regarding making the above a temporary format
default until astyle improves or a better replacement is available?
[Not] spending hours on merging branches can be a strong-enough
motivation to use less-than-perfect format above...
Or would it be better to post-process atyle-formatted code to untangle
#if FOO {
}
lines?
Cheers,
Alex.
Re: astyle
Alex Rousskov wrote:
On Mon, 2008-01-07 at 23:55 +0200, Tsantilas Christos wrote:
Alex Rousskov wrote:
On Sat, 2008-01-05 at 13:05 +0200, Tsantilas Christos wrote:
...
I suggest removing break-blocks both because of the above bug and
because it is trying to detect unrelated blocks, classes, etc. which
smells too much like AI to me.
The --brackets=linux (-l) option is useful though. Can you check whether
there is another --brackets option that works without the above bug and
does the subset of what --brackets=linux does?
We can only use the --brackets=break option which works well.
This option format the code as:
void Foo(bool isFoo)
{
if (isFoo)
{
bar();
}
else
{
anotherBar();
}
}
(This format is awful but considering the hours I spent to merge the
HEAD in async-calls ... it is the best :-) ! )
What do others think regarding making the above a temporary format
default until astyle improves or a better replacement is available?
[Not] spending hours on merging branches can be a strong-enough
motivation to use less-than-perfect format above...
Or would it be better to post-process atyle-formatted code to untangle
Less processing the better.
So putting up with a small ugly in exchange for a complicated hack is
okay by me.
On the bit-field problem, I have a similar mind. Even though the wrap is
extremely ugly the suggested fix makes code almost unreadable.
If we have to go the way of hacking bitfields around astyle, I would
suggest going to a macro (yuck). Like so:
#define BITFIELD(name,bits) unsigned int name : bits
struct {
BITFIELD(name, 1);
BITFIELD(flag, 1);
}
#if FOO {
}
lines?
Cheers,
Alex.
Amos
--
Please use Squid 2.6STABLE17 or 3.0STABLE1.
There are serious security advisories out on all earlier releases.
Re: astyle
On Tue, 2008-01-08 at 16:45 +1300, Amos Jeffries wrote:
On the bit-field problem, I have a similar mind. Even though the wrap is
extremely ugly the suggested fix makes code almost unreadable.
The fix (pre- and post-processing) is invisible to the programmer. It
would be auto-performed before and after astyle is run.
If we have to go the way of hacking bitfields around astyle, I would
suggest going to a macro (yuck). Like so:
#define BITFIELD(name,bits) unsigned int name : bits
struct {
BITFIELD(name, 1);
BITFIELD(flag, 1);
}
That could work indeed, provided astyle does not mangle the above into
some other ugly representation, especially if comments are added after
the declaration.
Thank you,
Alex.
Re: astyle
Alex Rousskov wrote:
On Tue, 2008-01-08 at 16:45 +1300, Amos Jeffries wrote:
On the bit-field problem, I have a similar mind. Even though the wrap is
extremely ugly the suggested fix makes code almost unreadable.
The fix (pre- and post-processing) is invisible to the programmer. It
would be auto-performed before and after astyle is run.
If we have to go the way of hacking bitfields around astyle, I would
suggest going to a macro (yuck). Like so:
#define BITFIELD(name,bits) unsigned int name : bits
struct {
BITFIELD(name, 1);
BITFIELD(flag, 1);
}
That could work indeed, provided astyle does not mangle the above into
some other ugly representation, especially if comments are added after
the declaration.
Cool. I'm building a little (so far) file of these tests so the styles
can be automatically verified.
If you or christos are doing the same could you commit it as a
test-suite file and test script?
Amos
--
Please use Squid 2.6STABLE17 or 3.0STABLE1.
There are serious security advisories out on all earlier releases.
astyle
On Sat, 2007-12-15 at 11:40 +1300, Amos Jeffries wrote: Alex Rousskov wrote: On Fri, 2007-12-14 at 21:35 +0100, Henrik Nordstrom wrote: On fre, 2007-12-14 at 18:26 +1300, Amos Jeffries wrote: He suggested a big(ger) delay than a few days to wait for bugs to prevent 'inventing two patches for two branches'. My opinion: With SQUID_3_0 branched there is no reason to hold back stuff from Squid-3 HEAD. Usually bugfixes is quite easy to back/forwardport, even if a lot has changed. Many of the more difficult bugs is found by testing new things. +1. My only regret is that I did not do the astyle check yet. If you can check what the latest astyle does to Squid3, please do that. I think having common automated format before the big commits would minimize formatting conflicts. Do you have a set of test-files to check this easily instead of a whole codebase manual check each time? Nope :-(. Generating a diff that excludes whitespace changes may be a good starting point. If we could fix code indentation alone, it would already be very useful, IMO. Alex.
Re: astyle
On Fri, 2007-12-14 at 23:17 +0100, Henrik Nordstrom wrote: On fre, 2007-12-14 at 14:36 -0700, Alex Rousskov wrote: My only regret is that I did not do the astyle check yet. If you can check what the latest astyle does to Squid3, please do that. I think having common automated format before the big commits would minimize formatting conflicts. Has there been any significant progress on astyle in the last year? I know they released new version(s), but I do not know whether the changes were significant enough to warrant its use for Squid3. I do agree that older astyle versions made Squid code look worse, not better (and even had bugs). Alex.
Re: astyle
On fre, 2007-12-14 at 14:36 -0700, Alex Rousskov wrote: My only regret is that I did not do the astyle check yet. If you can check what the latest astyle does to Squid3, please do that. I think having common automated format before the big commits would minimize formatting conflicts. Has there been any significant progress on astyle in the last year? Last time I looked it had a lot of trouble dealing with Squid-3, failing miserably on things like bitfields and several other constructs used. We had astyle as an automated check for some years, but gave up on it as it didn't improve the readability of the code, making more errors than it cleaned up. Regards Henrik signature.asc Description: This is a digitally signed message part
Re: astyle issues
I now have a simple patch for the bitfields issue. It turned out astyle
misread bitfield definitions as labels..
The block issue is not yet identified, but is not as irritating
either.
Regards
Henrik
sön 2003-02-23 klockan 00.33 skrev Robert Collins:
On Sun, 2003-02-23 at 02:12, Henrik Nordstrom wrote:
Robert Collins wrote:
I don't think this is a biggy: as we get more OOP, anonymous structs
will dissappear almost completely.
Not completely I think. Using anonymous structs is quite handy for
grouping related members.
, in C style coding yes. I'm pretty sure that over time they will
all go away - and in a sensible fashion.
Here is a example of a quite recently OOP class:
Please note that this is only 'partly' OOP. It's been factored out from
the global ACL class, and somewhat refactorded, but there is more that
can be done:
class ACLUserData : public ACLDatachar const *
{
[...]
SplayNodechar * *names;
struct
{
unsigned int case_insensitive:
1;
This flag should go away. Instead the Splay class should be a true
container that we simply insert, remove and find within. The container
will be parameterised for case sensitivity, and thus ACLUserData class
won't know whether it is case sensitive or not. For dumping the config
I'd like a utility class to store parse-once, forget forever options and
reduce the duplicated wordlist manipulation - and that class will know
about the -i.
unsigned int required:
1;
This option should go away completely. Instead we should replace the
ACLUserData instance with another that is also derived from ACLData, and
that implements the behaviour of REQUIRED. That will remove a bunch of
'if' clauses in the ACLUserData class.
}
flags;
private:
static MemPool *Pool;
};
We could rewrite this to drop any internal grouping of members within
classes, but I prefer not.
IMO it's not about dropping the grouping, its that the specific members
actually control behaviour - and that is almost always a perfect
scenario for the user of a strategy or state (the insensitive flag), and
for polymorphism (i.e. ACLRequiredDATA).
Rob
--
Henrik Nordstrom [EMAIL PROTECTED]
MARA Systems AB, Sweden
Re: astyle issues
Robert Collins wrote:
Should read:
struct
{
[...]
} Wais;
I don't think this is a biggy: as we get more OOP, anonymous structs
will dissappear almost completely.
Not completely I think. Using anonymous structs is quite handy for
grouping related members.
Here is a example of a quite recently OOP class:
class ACLUserData : public ACLDatachar const *
{
[...]
SplayNodechar * *names;
struct
{
unsigned int case_insensitive:
1;
unsigned int required:
1;
}
flags;
private:
static MemPool *Pool;
};
We could rewrite this to drop any internal grouping of members within
classes, but I prefer not.
Regards
Henrik
Re: astyle issues
On Sun, 2003-02-23 at 02:12, Henrik Nordstrom wrote:
Robert Collins wrote:
I don't think this is a biggy: as we get more OOP, anonymous structs
will dissappear almost completely.
Not completely I think. Using anonymous structs is quite handy for
grouping related members.
, in C style coding yes. I'm pretty sure that over time they will
all go away - and in a sensible fashion.
Here is a example of a quite recently OOP class:
Please note that this is only 'partly' OOP. It's been factored out from
the global ACL class, and somewhat refactorded, but there is more that
can be done:
class ACLUserData : public ACLDatachar const *
{
[...]
SplayNodechar * *names;
struct
{
unsigned int case_insensitive:
1;
This flag should go away. Instead the Splay class should be a true
container that we simply insert, remove and find within. The container
will be parameterised for case sensitivity, and thus ACLUserData class
won't know whether it is case sensitive or not. For dumping the config
I'd like a utility class to store parse-once, forget forever options and
reduce the duplicated wordlist manipulation - and that class will know
about the -i.
unsigned int required:
1;
This option should go away completely. Instead we should replace the
ACLUserData instance with another that is also derived from ACLData, and
that implements the behaviour of REQUIRED. That will remove a bunch of
'if' clauses in the ACLUserData class.
}
flags;
private:
static MemPool *Pool;
};
We could rewrite this to drop any internal grouping of members within
classes, but I prefer not.
IMO it's not about dropping the grouping, its that the specific members
actually control behaviour - and that is almost always a perfect
scenario for the user of a strategy or state (the insensitive flag), and
for polymorphism (i.e. ACLRequiredDATA).
Rob
--
GPG key available at: http://users.bigpond.net.au/robertc/keys.txt.
signature.asc
Description: This is a digitally signed message part
