Fix manual description in SSL_CTX_add_extra_chain_cert.3

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

Fix manual description in SSL_CTX_add_extra_chain_cert.3

kinichiro inoguchi
I think both SSL_CTX_get_extra_chain_certs and
SSL_CTX_get_extra_chain_certs_only should be described here.

ok?

Index: SSL_CTX_add_extra_chain_cert.3
===================================================================
RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3
--- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 -0000 1.7
+++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 -0000
@@ -115,7 +115,9 @@ An application should not free the
 object, nor the
 .Pf * Fa certs
 object retrieved by
-.Fn SSL_CTX_get_extra_chain_certs .
+.Fn SSL_CTX_get_extra_chain_certs
+and
+.Fn SSL_CTX_get_extra_chain_certs_only .
 .Sh RETURN VALUES
 These functions return 1 on success or 0 for failure.
 Check out the error stack to find out the reason for failure.

Reply | Threaded
Open this post in threaded view
|

Re: Fix manual description in SSL_CTX_add_extra_chain_cert.3

Ingo Schwarze
Hello Kinichiro-san,

Kinichiro Inoguchi wrote on Sun, Jan 12, 2020 at 05:09:52PM +0900:

> I think both SSL_CTX_get_extra_chain_certs and
> SSL_CTX_get_extra_chain_certs_only should be described here.

I think the text describing what to do with internal pointers
returned from LibreSSL functions is still quite a mess in general.
In some cases (like this one) something is said explicitly,
but there are many different wordings.  In other cases, nothing
is said explicitly.

In the case at hand, the sentence you are adding to feels somewhat
redundant because the main description already says "retrieves an
internal pointer".  The only reason i didn't delete the sentence
when last editing the page is because we didn't come round to a
general cleanup of such statements yet, and adding or deleting such
statements piece-meal without a comprehensive plan didn't feel like
a big improvement.

Regarding the particular change you propose, on top of the above,
i think what you are adding is already clear from the text above,
which explains the the pointers potentially returned from
SSL_CTX_get_extra_chain_certs_only(3) are a subset of those potentially
returned from SSL_CTX_get_extra_chain_certs(3).  So if the latter
mustn't be freed, that includes the former.

> ok?

I don't consider the patch a real improvement, but i don't strongly
object to it either.  What you are proposing to say is certainly
not wrong.


More generally, what do people think to switching to the following
concise wording:

  SOMEOBJ_add0(obj, p)  "transfers ownership of t to obj"
                        (or "sets the FOO of obj to p, transfering
                         ownership", depending on the context)
  SOMEOBJ_add1(obj, p)  "sets the FOO of obj to a (deep|shallow)
                         copy of p"
  p = SOMEOBJ_get0(obj) "retrieves an internal pointer to the FOO of obj"
  p = SOMEOBJ_get1(obj) "returns a (deep|shallow) copy of the FOO of obj"

And then delete all the repetitive wordings like "you must free"
or "you must not free"?

Yours,
  Ingo


> Index: SSL_CTX_add_extra_chain_cert.3
> ===================================================================
> RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v
> retrieving revision 1.7
> diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3
> --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 -0000 1.7
> +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 -0000
> @@ -115,7 +115,9 @@ An application should not free the
>  object, nor the
>  .Pf * Fa certs
>  object retrieved by
> -.Fn SSL_CTX_get_extra_chain_certs .
> +.Fn SSL_CTX_get_extra_chain_certs
> +and
> +.Fn SSL_CTX_get_extra_chain_certs_only .
>  .Sh RETURN VALUES
>  These functions return 1 on success or 0 for failure.
>  Check out the error stack to find out the reason for failure.

Reply | Threaded
Open this post in threaded view
|

Re: Fix manual description in SSL_CTX_add_extra_chain_cert.3

kinichiro inoguchi
Hello Ingo-san,

Thanks for your answer.
Today, I had read through this manual, and I just thought that the newly added
function was missed in the description section at last update.
But now I had understood the status of manual maintenance by your explanation.
I just read only one manual page and had not took consider about others.
I should have thought about this from comprehensive perspective of view.
I would like to abandon this patch for now.

Regards,
Kinichiro Inoguchi

On Sun, Jan 12, 2020 at 10:46:26AM +0100, Ingo Schwarze wrote:

> Hello Kinichiro-san,
>
> Kinichiro Inoguchi wrote on Sun, Jan 12, 2020 at 05:09:52PM +0900:
>
> > I think both SSL_CTX_get_extra_chain_certs and
> > SSL_CTX_get_extra_chain_certs_only should be described here.
>
> I think the text describing what to do with internal pointers
> returned from LibreSSL functions is still quite a mess in general.
> In some cases (like this one) something is said explicitly,
> but there are many different wordings.  In other cases, nothing
> is said explicitly.
>
> In the case at hand, the sentence you are adding to feels somewhat
> redundant because the main description already says "retrieves an
> internal pointer".  The only reason i didn't delete the sentence
> when last editing the page is because we didn't come round to a
> general cleanup of such statements yet, and adding or deleting such
> statements piece-meal without a comprehensive plan didn't feel like
> a big improvement.
>
> Regarding the particular change you propose, on top of the above,
> i think what you are adding is already clear from the text above,
> which explains the the pointers potentially returned from
> SSL_CTX_get_extra_chain_certs_only(3) are a subset of those potentially
> returned from SSL_CTX_get_extra_chain_certs(3).  So if the latter
> mustn't be freed, that includes the former.
>
> > ok?
>
> I don't consider the patch a real improvement, but i don't strongly
> object to it either.  What you are proposing to say is certainly
> not wrong.
>
>
> More generally, what do people think to switching to the following
> concise wording:
>
>   SOMEOBJ_add0(obj, p)  "transfers ownership of t to obj"
>                         (or "sets the FOO of obj to p, transfering
>                          ownership", depending on the context)
>   SOMEOBJ_add1(obj, p)  "sets the FOO of obj to a (deep|shallow)
>                          copy of p"
>   p = SOMEOBJ_get0(obj) "retrieves an internal pointer to the FOO of obj"
>   p = SOMEOBJ_get1(obj) "returns a (deep|shallow) copy of the FOO of obj"
>
> And then delete all the repetitive wordings like "you must free"
> or "you must not free"?
>
> Yours,
>   Ingo
>
>
> > Index: SSL_CTX_add_extra_chain_cert.3
> > ===================================================================
> > RCS file: /cvs/src/lib/libssl/man/SSL_CTX_add_extra_chain_cert.3,v
> > retrieving revision 1.7
> > diff -u -p -u -p -r1.7 SSL_CTX_add_extra_chain_cert.3
> > --- SSL_CTX_add_extra_chain_cert.3 2 Jan 2020 09:09:16 -0000 1.7
> > +++ SSL_CTX_add_extra_chain_cert.3 12 Jan 2020 08:06:49 -0000
> > @@ -115,7 +115,9 @@ An application should not free the
> >  object, nor the
> >  .Pf * Fa certs
> >  object retrieved by
> > -.Fn SSL_CTX_get_extra_chain_certs .
> > +.Fn SSL_CTX_get_extra_chain_certs
> > +and
> > +.Fn SSL_CTX_get_extra_chain_certs_only .
> >  .Sh RETURN VALUES
> >  These functions return 1 on success or 0 for failure.
> >  Check out the error stack to find out the reason for failure.