Add missing 'RETURN VALUES' sections in doc

[InfoHunter]

All missing ones are added.

Checklist

• documentation is added or updated

[dot-asm]

Missing stop.

[dot-asm]

This sounds like different negative values would indicate different error codes, but apparently it's just negative value to signal an error condition...

[dot-asm]

On side note one can wonder if it's appropriate to elaborate on length units. I mean that it's buffer length, not character count...

[dot-asm]

One can wonder if it would be appropriate to reword the description. I mean it doesn't actually return a mask, but rather a union [determined by masking specific flags]...

[dot-asm]

Hmm.... I'm struggling with "a null-terminated string" here. I mean "a" kind of implies "some", while it's quite specific. In DES_fcrypt case it's either buffer supplied by caller or NULL, and in DES_crypt - static buffer or NULL.

[On side note one can wonder if it would be appropriate to not modify buffer if error is returned. I mean DES_fcrypt modifies buffer in case error is returned, and maybe it shouldn't. Once again, this is side note.]

[InfoHunter]

Or could we say 'null terminated buffer' or something like that?

[dot-asm]

Just in case of misunderstanding, I'm struggling specifically with "a", i.e. with it being indefinite article. And underlying question rather is if one should be more specific than referring to unnamed buffer, with or without article. After all, description says nothing about return value, only input arguments, so it might be appropriate to say explicitly how it is. So that reader doesn't have to consult source code...

[InfoHunter]

How about this:

DES_fcrypt() and DES_crypt() return NULL if an error occurred. On success, DES_fcrypt() returns a null-terminated string in user provided buffer and DES_crypt() returns a null-terminated string in a static buffer.

[dot-asm]

Well, to me it appears more of a function description than description of return value. I mean part of placing null-terminated string to a buffer. Consider 'man strcpy' for example... So I'd say it should rather read "On success DES_fcrypt() returns pointer to the caller-provided buffer, and DES_crypt() - to a static buffer."

Or related side note one can wonder if DESCRIPTION part needs some adjustments. Most notably mention that returned string is null-terminated. [On related note one can also wonder if "this version takes only a small amount of space" "bragging" clause is actually relevant today.]

[InfoHunter]

This is fixed and the 'null-terminated' description is added to section DESCRIPTION.

[dot-asm]

I'd rather say "requested [immutable?] version string". I mean it has lesser to do with format, but different information...

[dot-asm]

Looks like it's "(NONE)", not "(NULL)".

[dot-asm]

"NID values" stings my eye a little, but that might be just me. I mean ping native English speakers if it should be "NID value"...

[t-j-h]

NID value - it is a singular return.

[dot-asm]

I'd write explicitly "extra data pointer" or "application-supplied extra data"... No?

[dot-asm]

So far functions were listed without arguments, i.e. as UI_process()...

[dot-asm]

On common note that there are more things that "sting" my eye. So common ping to native English speakers...

[InfoHunter]

Addressed almost all comments and left some comments for native English speakers either...

[dot-asm]

In such case "flag combination presenting the cause"... Though "representing" might be better...

[InfoHunter]

Fixed

[InfoHunter]

Fixed

…

On 28 Dec 2017, at 16:06, Tim Hudson ***@***.***> wrote: @t-j-h commented on this pull request. In doc/man3/SSL_CIPHER_get_name.pod <#4976 (comment)>: > @@ -150,6 +150,31 @@ Some examples for the output of SSL_CIPHER_description(): ECDHE-RSA-AES256-GCM-SHA256 TLSv1.2 Kx=ECDH Au=RSA Enc=AESGCM(256) Mac=AEAD RSA-PSK-AES256-CBC-SHA384 TLSv1.0 Kx=RSAPSK Au=RSA Enc=AES(256) Mac=SHA384 +=head1 RETURN VALUES + +SSL_CIPHER_get_name(), SSL_CIPHER_standard_name(), OPENSSL_cipher_name(), +SSL_CIPHER_get_version() and SSL_CIPHER_description() return the corresponding +value in null-terminated string for a specific cipher or "(NULL)" +if the cipher is not found. + +SSL_CIPHER_get_bits() returns a positive integer or 0 if an error occurred. + +SSL_CIPHER_get_cipher_nid(), SSL_CIPHER_get_digest_nid(), +SSL_CIPHER_get_kx_nid() and SSL_CIPHER_get_auth_nid() return NID values or +B<NID_undef> if an error occurred. NID value - it is a singular return. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4976 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwyxouhroQcgi7aTWfDvAo5E3TRrzAnks5tE0wAgaJpZM4RMObi>.

[mattcaswell]

This is an awesome piece of work!

[mattcaswell]

"returns the number of bytes"

[mattcaswell]

Should we say something about who is responsible for freeing this data?

[InfoHunter]

This is described in the DESCRIPTION section, then still need to re-describe here?

[mattcaswell]

Ok - no need if we say it elsewhere.

[mattcaswell]

"according to whether B<a> is greater than..."

[mattcaswell]

"return the number"

[mattcaswell]

Should "BIO method" be written as "BIO_METHOD"?

[mattcaswell]

"return the number of"

[mattcaswell]

Preferred is to avoid contractions: it's => it is (and below)

[mattcaswell]

typo: success

[mattcaswell]

or equal to

[mattcaswell]

typo: occurred

[richsalz]

First off, @InfoHunter, congratulations and thanks for doing this!
Second, could you think about adding this diff?

diff --git a/util/find-doc-nits b/util/find-doc-nits
index 7087d366d2..8d580de045 100755
--- a/util/find-doc-nits
+++ b/util/find-doc-nits
@@ -24,7 +24,6 @@ our($opt_h);
 our($opt_l);
 our($opt_n);
 our($opt_p);
-our($opt_s);
 our($opt_u);
 our($opt_c);
 
@@ -35,7 +34,6 @@ Find small errors (nits) in documentation.  Options:
     -d Detailed list of undocumented (implies -u)
     -l Print bogus links
     -n Print nits in POD pages
-    -s Also print missing sections in POD pages (implies -n)
     -p Warn if non-public name documented (implies -n)
     -u List undocumented functions
     -h Print this help message
@@ -217,7 +215,6 @@ sub check()
 
     foreach ((@{$mandatory_sections{'*'}}, @{$mandatory_sections{$section}})) {
         # Skip "return values" if not -s
-        next if $_ eq 'RETURN VALUES' and not $opt_s;
         print "$id: missing $_ head1 section\n"
             if $contents !~ /^=head1\s+${_}\s*$/m;
     }
@@ -474,13 +471,13 @@ sub checkflags() {
     return $ok;
 }
 
-getopts('cdlnsphu');
+getopts('cdlnphu');
 
 &help() if $opt_h;
-$opt_n = 1 if $opt_s or $opt_p;
+$opt_n = 1 if $opt_p;
 $opt_u = 1 if $opt_d;
 
-die "Need one of -[cdlnspu] flags.\n"
+die "Need one of -[cdlnpu] flags.\n"
     unless $opt_c or $opt_l or $opt_n or $opt_u;
 
 if ( $opt_c ) {

[InfoHunter]

Second, could you think about adding this diff?

Yes, glad to do so.

[InfoHunter]

All unnecessary 'void' return values are removed. But some ones are left there if all functions in a POD file are all 'void' functions - it's necessary to put words in the section otherwise the section will be empty which is not valid for doc-nits checking.

[dot-asm]

It looks like I was concentrating on wrong "a", huh? I mean initially I was tipped off by "a string", but it's still "a pointer", but to "the buffer". Sorry about possible confusion...

[InfoHunter]

I don't quite get the point, could you explain a bit more? Seems it's fine saying 'a point to the buffer'....

[dot-asm]

Point is that my English sucks. I mean originally I made fuzz about "a" being indefinite article as if it has no bearing here. But it apparently does, "a pointer to the buffer"... Oh well... It was inessential comment...

[InfoHunter]

Ahh...

[dot-asm]

I find this a bit ambiguous. NULL customarily refers to NULL pointer, while we are obviously talking about character here. In other words "terminated by null character" would presumably be better. And I wouldn't be surprised if it should read "terminated by a null character"...

[kaduk]

ascii(7) clearly shows the value 0 as NUL '\0' (null character)

[dot-asm]

Yeah, but NULL, i.e. with double L, still has quite too strong pointer "clang"... In other words "NUL character" would also do, "NULL" alone ... not so much....

[richsalz]

Yes, NULL means pointer and NUL means \0 byte. It would be nice to fix this, or a new PR.

[InfoHunter]

Fixed in this PR

[dot-asm]

I realize that "BIO method function" is taken from description that was there earlier, but I find the term confusing. I mean it doesn't actually return a function, but rather a dispatch table. Thoughts?

[richsalz]

How about just remove "function":

memroy BIO method, a structure of function pointers

[InfoHunter]

That structure contains other data than function pointers...

[InfoHunter]

It might be better by just saying 'return a valid memory B<BIO_METHOD> structure'...

[FdaSilvaYY]

s/vaild/valid

[InfoHunter]

Fixed

[FdaSilvaYY]

s/occured/occurred

[InfoHunter]

Fixed

[FdaSilvaYY]

s/poisitve/positive

[InfoHunter]

Fixed

[dot-asm]

On more essential note could you clarify what needs to be squashed. It looks like everything except perhaps find-doc-nits? Maybe find-doc-nits should even be standalone? Or shall we just squash everything?

[InfoHunter]

Yes, I think all commits should be squashed into one commit except the 'find-doc-nits' commit - which is 2015e92 exactly...

[mattcaswell]

Grrr....I tried to merge this but there are conflicts with master and it now needs a rebase.

[InfoHunter]

@mattcaswell rebase is done and I've squashed all necessary commits.

[InfoHunter]

Errr, I just noticed seems all changed files' copyright date should be updated to end with 2018, would you guys like to do this in this PR or in a separate one?

[mattcaswell]

Lets do it in this one - if you don't mind doing the updates?

[InfoHunter]

Don't mind and a new commit is added to update the dates - which seems no need to be squashed...

[mattcaswell]

As the new commit is trivial I am assuming @dot-asm's approval still applies and will merge shortly.

[mattcaswell]

Pushed. Thanks.

[richsalz]

@InfoHunter , this was a tour de force PR. Thank you!

[InfoHunter]

Thanks!