Revert commit 10621ef white space nightmare

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Revert commit 10621ef white space nightmare

Leonard den Ottolander
Hello,

https://github.com/openssl/openssl/commit/10621efd3296a92f489f6ab26a88e88d9790930e#diff-4b59eddb1c722b1dc3d17b5f64149e12

is a white space nightmare. The replacement of "#define"s by "# define"s
etc. is just silly and makes it unnecessarily hard to port patches
between different releases (working with RHEL 7.3 "hobbled 1.0.1e" vs.
1.0.1u). Sadly patch -l chokes on this kind of white space nonsense and
who can blame it?

If one wants to indent directives space is normally inserted before the
hash sign. I don't remember ever seeing directives being indented by
adding white space between the hash sign and the directive.

Please revert this commit (in all branches), even though it has been a
while. Thank you.

Regards,
Leonard.


--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Revert commit 10621ef white space nightmare

Salz, Rich
Sorry you feel this way, but the patch is not being reverted.  First of all, 1.0.1 is now end of life and gets no updates :) As for the specific pre-processor, there are systems out there that only recognized the poundsign if it was in the first column (silly but true).  Also, we prefer the whitespace on CPP lines the way we have it.  You can find some details about the code reformat at two blog entries: https://www.openssl.org/blog/blog/2015/01/05/source-code-reformat/ and https://www.openssl.org/blog/blog/2015/02/11/code-reformat-finished/ 

The best way to address your issue is to start using 1.0.2 or later.  If you cannot do that, then a perl script like this can help:
        perl -pi.bak -e 's/^\s*#\s*/#/' files...

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Revert commit 10621ef white space nightmare

Matt Caswell-2
In reply to this post by Leonard den Ottolander


On 09/01/17 17:46, Leonard den Ottolander wrote:

> Hello,
>
> https://github.com/openssl/openssl/commit/10621efd3296a92f489f6ab26a88e88d9790930e#diff-4b59eddb1c722b1dc3d17b5f64149e12
>
> is a white space nightmare. The replacement of "#define"s by "# define"s
> etc. is just silly and makes it unnecessarily hard to port patches
> between different releases (working with RHEL 7.3 "hobbled 1.0.1e" vs.
> 1.0.1u). Sadly patch -l chokes on this kind of white space nonsense and
> who can blame it?
>
> If one wants to indent directives space is normally inserted before the
> hash sign. I don't remember ever seeing directives being indented by
> adding white space between the hash sign and the directive.
>
> Please revert this commit (in all branches), even though it has been a
> while. Thank you.

That particular commit was the result of a lot work and discussion on
this list and in other places. This is the code reformat commit and
changes the format of the source to be consistent with the OpenSSL
coding style:

https://www.openssl.org/policies/codingstyle.html

Like I said there was a lot of discussion at the time with arguments in
favour and against the reformat - although the on the whole most people
were in favour.

The coding style as a whole was based heavily on the Linux Kernel coding
style. I don't recall the provenance of the pre-processor indentation
style. Needless to say though everyone has their particular thoughts
about what makes "good" style - and no matter what you come up with no
one is going to be pleased with all of it.

Personally I believe the reformat has done nothing but good for us. It
has made maintaining the source code *far* easier.

Whatever your thoughts on it a lot of water has gone under the bridge
since then and there have been a lot of commits to all the branches.
Reverting this patch is just not an option at this stage even if we
wanted to (which we don't) - you would disappear into conflict hell
never to return.

Of course 1.0.1 is now out of support anyway so we won't be making any
more commits to that particular branch!

Matt

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Revert commit 10621ef white space nightmare

Claus Assmann-2
In reply to this post by Leonard den Ottolander
On Mon, Jan 09, 2017, Leonard den Ottolander wrote:

> If one wants to indent directives space is normally inserted before the
> hash sign. I don't remember ever seeing directives being indented by
> adding white space between the hash sign and the directive.

Then you didn't look at source code for
sendmail, libesmtp, deliver, mutt, nail, afl, flex, bison, make,
(just some stuff I have available for grep '^#  *[dei]')
and many others...

BTW: the reason the spaces are after '#' is because some (old?)
pre-preprocessors handle '#' only in the first column.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Revert commit 10621ef white space nightmare

John Denker-2
In reply to this post by Leonard den Ottolander
On 01/09/2017 10:46 AM, Leonard den Ottolander wrote:

> I don't remember ever seeing directives being indented by adding
> white space between the hash sign and the directive.

In my world, that is quite common.

> If one wants to indent directives space is normally inserted before
> the hash sign.

No, that is not normal.  It is not even permitted by traditional
versions of the preprocessor.  I quote from
  https://gcc.gnu.org/onlinedocs/gcc-3.1/cpp/Traditional-Mode.html

>> Preprocessing directives are recognized in traditional C only when
>> their leading # appears in the first column. There can be no
>> whitespace between the beginning of the line and the #.

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Revert commit 10621ef white space nightmare

Leonard den Ottolander
In reply to this post by Matt Caswell-2
Hello Matt,

On Mon, 2017-01-09 at 18:06 +0000, Matt Caswell wrote:
> That particular commit was the result of a lot work and discussion on
> this list and in other places. This is the code reformat commit and
> changes the format of the source to be consistent with the OpenSSL
> coding style:
>
> https://www.openssl.org/policies/codingstyle.html

Yeah I sort of guessed that but wasn't around when that discussion took
place. I'll have a look at that guide and will learn to live with
it ;) . With some hand editing I got my patch fixed as you can see from
my previous post.

Regards,
Leonard.


--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev