On 04/11/2013 06:59 PM, Rich Felker wrote:
> On Thu, Apr 11, 2013 at 06:42:26PM -0400, Zvi Gilboa wrote:
>> Greetings,
>>
>> Looking at crypt.c and crypt.h, I was wondering whether having
>> different-size /crypt_data/ structures was intentional, and if so,
>> for what reason.  At the moment, the declarations are (and see also
>> the attached code):
> Yes and no. struct crypt_data becomes part of the ABI of the program
> that calls crypt_r, so it needs to be "large enough for any future
> password hash". The number 256 was chosen as a nice round value. On
> the other hand, the static buffer used by crypt() only needs to be
> "large enough for any currently-supported password hash", which is
> presently 128.
>
>> /<crypt.h>/
>> struct crypt_data {
>>      int initialized;
>>      char *__buf[256]*;
>> }; /* 260 bytes when sizeof(int)==4 */
>>
>> /<crypt.c>/
>> char *__crypt_r(const char *, const char *, *struct crypt_data **);
>>
>> char *crypt(const char *key, const char *salt)
>> {
>>      static char *buf[128]*;
>>      return __crypt_r(key, salt, *(struct crypt_data *)buf*);
>>      /* when sizeof(int)==4, this leaves __buf with 124 bytes. */
> No, the size is the full 128. There really is no such member as
> "initialized" as far as musl's __crypt_r is concerned; the whole
> object is used simply as a char[] buffer. The only reason the
> "initialized" member exists is to meet the API requirements of the
> crypt_r function -- per the API, applications are supposed to either
> set crypt_data.initialized=0, or zero-initialize the whole object,
> before calling crypt_r. If a member named "initialized" did not exist,
> such programs would fail to compile. However, musl itself has no use
> for any initialized member, since it only uses crypt_data as an output
> buffer, not a working buffer for crypto state.
>
>> On that note: since the /initialized /member is (currently) never
>> referenced by name, adding a comment about that to the code might
>> help readers who are yet to be initiated:)
> Where do you think would be best to add it? In __crypt_r? The
> "natural" place would be the headers, but the current policy is to
> avoid comments in the headers, especially ones which would be
> implementation-specific rather than documenting the API.

Thanks for the explanation!  Since crypt.c is likely to be visited by 
anyone going through the crypt-related code, I'd suggest placing the 
comment there (proposed comment attached).

>
> Rich