Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 11 Apr 2013 22:18:00 -0400
From: Zvi Gilboa <zg7s@...rvices.virginia.edu>
To: <musl@...ts.openwall.com>
Subject: Re: crypt_data: structure size in crypt.c vs. crypt.h

On 04/11/2013 09:42 PM, Rich Felker wrote:
> On Thu, Apr 11, 2013 at 08:03:15PM -0400, Zvi Gilboa wrote:
>> 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).
>>
>> [...]
>> >From b9d9d4c46486b4f0e77cf7b4455de8ba87c9f5ef Mon Sep 17 00:00:00 2001
>> From: Zvi Gilboa <zgilboa@...lboa-S14Y-linux.(none)>
>> Date: Thu, 11 Apr 2013 19:52:05 -0400
>> Subject: [PATCH] Committer: Zvi Gilboa <zgilboa@...lboa-S14Y-linux.(none)>
>>
>> On branch crypt_data_comment
>> ---
>>   src/crypt/crypt.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/src/crypt/crypt.c b/src/crypt/crypt.c
>> index f1e310f..fdc4427 100644
>> --- a/src/crypt/crypt.c
>> +++ b/src/crypt/crypt.c
>> @@ -1,6 +1,12 @@
>>   #include <unistd.h>
>>   #include <crypt.h>
>>   
>> +/***
>> +  struct crypt_data       --> the entire structure is used as a char[] buffer
>> +        int initialized   --> member exists to satisfy API requirements, currently not used
>> +        char __buf[256];  --> large enough for any future password hash
>> + ***/
>> +
> I understand your motivation, but I don't want to introduce comments
> like this that duplicate definitions from elsewhere. I think the
> comments should be in crypt.c and crypt_r.c, and should be something
> along the line of:
>
> (crypt.c) This buffer is sufficiently large for all
> currently-supported hash types. It needs to be updated if longer
> hashes are added. The cast to struct crypt_data * is purely to meet
> the public API requirements of the crypt_r function; the
> implementation of crypt_r uses the object purely as a char buffer.
>
> (crypt_r.c) Per the crypt_r API, the caller has provided a pointer to
> struct crypt_data; however, this implementation does not use the
> structure to store any internal state, and treats it purely as a char
> buffer for storing the result.
>
> Would these comments have been sufficient for you to understand what
> was going on when you first read the source? If so, I'll go ahead and
> add them; if not, what would you like to see improved?

Thank you, Rich, the above would have made things perfectly clear -- and 
hopefully would benefit future readers/porters as well. And while we are 
at it...

-- has there ever been an attempt (or an intention) to create scripts 
that would turn the source tree into a "book" (a la /literate 
programming/)?  The lightness of /nuweb/ (http://nuweb.sourceforge.net/) 
should make it a perfect match for musl, and it should not be too 
complicated to create some "reverse literate programming" scripts 
(reverse, since in "straight" literate programming the C course files 
are the output, whereas here they will be used as the input).

-- any general interest in a command-line utility that provides the 
Wiki-based build status of a specific package?  I have just create one, 
and will be happy to "generalize" it.

>
> Rich


Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.