Freeing of const char * fields in ASN1_OBJECT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

Many thanks for the extensive explanation, I understand the rationale now.

> Now, a *good* public C library will implement proper ADTs.

Indeed I am used to using ADT mechanisms so I did not run into this
issue anywhere else before. It will be a lot of work to make all types
abstracted in OpenSSL... However, as a compromise, I wonder how much
work it would be to at least provide encapsulating allocator/initializer
and free functions for all datatypes. In that case, the const qualifier
for some fields in the struct would make complete sense and the fact
that the const field is allocated/freed by the library is then no longer
anybody else's business.

My problem was indeed resolved by using OBJ_create(), so from now on I
will be looking for creation functions rather than assuming that I am
supposed/allowed to create my own structs directly.

Reinier

PS: apparently there are more reasons to cast away const-ness; I ran
into the following in objects/obj_lib.c:

 65 ASN1_OBJECT *OBJ_dup(const ASN1_OBJECT *o)
 66         {
 67         ASN1_OBJECT *r;
 68         int i;
 69         char *ln=NULL,*sn=NULL;
 70         unsigned char *data=NULL;
 71
 72         if (o == NULL) return(NULL);
 73         if (!(o->flags & ASN1_OBJECT_FLAG_DYNAMIC))
 74                 return((ASN1_OBJECT *)o); /* XXX: ugh! Why? What kind of
 75                                              duplication is this??? */

On 10/28/15 11:02 PM, Michael Wojcik wrote:
>> From: openssl-users [mailto:openssl-users-bounces at openssl.org] On Behalf
>> Of Reinier Torenbeek
>> Sent: Wednesday, October 28, 2015 16:55
>>
>> In asn1/asn1.h, the fields sn and ln in ASN1_OBJECT are defined as const
>> char *:
>>
>> 211 typedef struct asn1_object_st
>> 212         {
>> 213         const char *sn,*ln;
>> 214         int nid;
>> 215         int length;
>> 216         const unsigned char *data;      /* data remains const after
>> init */
>> 217         int flags;      /* Should we free this one */
>> 218         } ASN1_OBJECT;
>>
>> but in asn1/a_object.c, the get casted to void * and freed:
>> ...
>>
>> Given that a lot of the code is supposed to be self-describing (due to
>> lack of documentation), it is somewhat disturbing that I can not rely on
>> the const qualifiers to be honored. Any thoughts on this?
> It's a well-known problem with the semantics of const in C.
>
> The parameter to free() cannot have the const qualifier, because the semantics of free say that the object referred to by the pointer reaches the end of its lifetime during the call to free(), and attempting to use it after that results in Undefined Behavior.
>
> However, it is perfectly reasonable to want to allocate storage, initialize (and even alter) its contents, but have that object treated only as an rvalue elsewhere. This is a very typical use case for a library or other code that presents an ADT (abstract data type) to a caller.
>
> The mechanism for informing a caller that an object should not be modified is the const qualifier.
>
> There are only two ways around this problem. One is for the ADT implementation to keep both a const and a non-const pointer to the same area, publish the const one, and use the non-const one for freeing. That's a rather absurd duplication of effort (and a small amount of storage) just to account for C's simplistic const/free semantics.
>
> The other way, which is the one everyone uses, is to cast away the constness of the pointer when passing it to free.
>
> Note that the problem arises in other contexts. That's why C++ has a category of casts just for this purpose (casting away constness). Another case you often see is a function in an external library that never modifies the object pointed to by a parameter, but failed to declare it as const (generally because it was written by someone who doesn't understand C, which is to say ~97% of all C programmers).
>
> As a rule of thumb, when a function parameter itself is a pointer to a const-qualified object type (this doesn't apply to const pointers to non-const objects), you should be able to assume that the function does not change the object. When a function parameter is a pointer to a non-const-qualified aggregate type (struct or union) that happens to include const-qualified members, do NOT assume that there are no cases in which those objects are modified. You really have to check the implementation of that function.
>
> C function contracts are weak.
>
> Now, a *good* public C library will implement proper ADTs. It will define them as pointers to incomplete structures, so you can't go poking around inside them; it will give you construction and cleanup functions; and it will give you accessor functions. Some people will complain that at that point you're no longer writing C - you're reinventing C++ - to which I'd reply that they don't really understand C or C++.
>




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux