[openssl-dev] Under-utilization of "const" in prototyping?

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

[openssl-dev] Under-utilization of "const" in prototyping?

Philip Prindeville
Hi all,

I was looking at some functions like ASN1_digest() or macros like
TYPEDEF_I2D_OF() and wondering why more of the parameters that are
unmodified aren't marked as "const", for instance "data" in
ASN1_digest() and "type" in TYPEDEF_I2D_OF().

I started to think about doing a scrub of this, but it likely couldn't
be done incrementally: there's too much interdependence in some of the code.

It would best be done as "throwing a switch".

How likely is this?

In most cases, changing a parameter to "const" in the API shouldn't
affect user code, so this wouldn't break existing source using OpenSSL.

Looks like all of the code could be changed in a couple of man-days.

Is it worthwhile doing?

-Philip

_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Kurt Roeckx
On Thu, Dec 04, 2014 at 05:22:02PM -0700, Philip Prindeville wrote:

> Hi all,
>
> I was looking at some functions like ASN1_digest() or macros like
> TYPEDEF_I2D_OF() and wondering why more of the parameters that are
> unmodified aren't marked as "const", for instance "data" in ASN1_digest()
> and "type" in TYPEDEF_I2D_OF().
>
> I started to think about doing a scrub of this, but it likely couldn't be
> done incrementally: there's too much interdependence in some of the code.
>
> It would best be done as "throwing a switch".
>
> How likely is this?
>
> In most cases, changing a parameter to "const" in the API shouldn't affect
> user code, so this wouldn't break existing source using OpenSSL.
>
> Looks like all of the code could be changed in a couple of man-days.
>
> Is it worthwhile doing?

I would like to see such changes.  We've been slowly adding more
const, but it could always use more of them.  So feel free to send
a patch.


Kurt

_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Philip Prindeville

On 12/04/2014 05:30 PM, Kurt Roeckx wrote:

> On Thu, Dec 04, 2014 at 05:22:02PM -0700, Philip Prindeville wrote:
>> Hi all,
>>
>> I was looking at some functions like ASN1_digest() or macros like
>> TYPEDEF_I2D_OF() and wondering why more of the parameters that are
>> unmodified aren't marked as "const", for instance "data" in ASN1_digest()
>> and "type" in TYPEDEF_I2D_OF().
>>
>> I started to think about doing a scrub of this, but it likely couldn't be
>> done incrementally: there's too much interdependence in some of the code.
>>
>> It would best be done as "throwing a switch".
>>
>> How likely is this?
>>
>> In most cases, changing a parameter to "const" in the API shouldn't affect
>> user code, so this wouldn't break existing source using OpenSSL.
>>
>> Looks like all of the code could be changed in a couple of man-days.
>>
>> Is it worthwhile doing?
> I would like to see such changes.  We've been slowly adding more
> const, but it could always use more of them.  So feel free to send
> a patch.
>
>
> Kurt
>

I'll make a deal: if you can suggest a way to carve up the task so that
it *can* be done incrementally, I'll see what I can do.

For kicks and giggles, I tried to change crypto/asn1/asn1.h and the
functions it contains, and it quickly rippled out into other modules
(like RSA, etc).

-Philip

_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Daniel Kahn Gillmor
On 12/04/2014 07:34 PM, Philip Prindeville wrote:
> I'll make a deal: if you can suggest a way to carve up the task so that
> it *can* be done incrementally, I'll see what I can do.

My experience with trying to constify larger, more-complex codebases is
that it is often *very* difficult to do incrementally.

If you have a way to do static analysis to find leaf functions, that
might be a way to get some smaller pieces that you can constify, but i
don't know the common static analysis tools well enough to be able to
point you to such a thing.

> For kicks and giggles, I tried to change crypto/asn1/asn1.h and the
> functions it contains, and it quickly rippled out into other modules
> (like RSA, etc).

I think sometimes just diving in like this is worth doing.  A successful
constifying rampage across the OpenSSL codebase would be real boon to
the project (of course it would probably end up modifying some public
interfaces, so it would need to take effect publicly during an API change).

        --dkg


_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev

signature.asc (968 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Kurt Roeckx
On Fri, Dec 05, 2014 at 10:40:07AM -0500, Daniel Kahn Gillmor wrote:
> (of course it would probably end up modifying some public
> interfaces, so it would need to take effect publicly during an API change).

Adding const to a function shouldn't be a problem for C.  It would
be for C++.


Kurt

_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Viktor Dukhovni
On Fri, Dec 05, 2014 at 05:07:04PM +0100, Kurt Roeckx wrote:

> On Fri, Dec 05, 2014 at 10:40:07AM -0500, Daniel Kahn Gillmor wrote:
> > (of course it would probably end up modifying some public
> > interfaces, so it would need to take effect publicly during an API change).
>
> Adding const to a function shouldn't be a problem for C.  It would
> be for C++.

Actually, it can be a problem with C also.  Replacing "char **" in
a function prototype with "const char **" changes the API.

Postfix has #ifdefs for a previous "constification" effort in OpenSSL:

    SSL_SESSION *tls_session_activate(const char *session_data, int session_data_len)
    {
    #if (OPENSSL_VERSION_NUMBER < 0x0090707fL)
    #define BOGUS_CONST
    #else
    #define BOGUS_CONST const
    #endif
        SSL_SESSION *session;
        BOGUS_CONST unsigned char *ptr;

        /*
         * Activate the SSL_SESSION object.
         */
        ptr = (BOGUS_CONST unsigned char *) session_data;
        session = d2i_SSL_SESSION((SSL_SESSION **) 0, &ptr, session_data_len);
        if (!session)
            tls_print_errors();

        return (session);
    }

--
        Viktor.
_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Kurt Roeckx
On Fri, Dec 05, 2014 at 05:15:24PM +0000, Viktor Dukhovni wrote:

> On Fri, Dec 05, 2014 at 05:07:04PM +0100, Kurt Roeckx wrote:
>
> > On Fri, Dec 05, 2014 at 10:40:07AM -0500, Daniel Kahn Gillmor wrote:
> > > (of course it would probably end up modifying some public
> > > interfaces, so it would need to take effect publicly during an API change).
> >
> > Adding const to a function shouldn't be a problem for C.  It would
> > be for C++.
>
> Actually, it can be a problem with C also.  Replacing "char **" in
> a function prototype with "const char **" changes the API.

This is actually in the C FAQ:
http://c-faq.com/ansi/constmismatch.html


Kurt

_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Salz, Rich
In reply to this post by Viktor Dukhovni
Gee, BOGUS_CONST is a little harsh.  MAYBE_CONST or SHOULDBE_CONST would have been nicer.
_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: [openssl-dev] Under-utilization of "const" in prototyping?

Viktor Dukhovni
On Fri, Dec 05, 2014 at 12:28:29PM -0500, Salz, Rich wrote:

> Gee, BOGUS_CONST is a little harsh.  MAYBE_CONST or SHOULDBE_CONST would have been nicer.

Wietse does not mince words, and sarcasm is often lost in translation.

--
        Viktor.
_______________________________________________
openssl-dev mailing list
[hidden email]
https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev