On 04/07/2014 02:20 PM, Dmitri Pal wrote:
> On 04/07/2014 05:47 AM, Lukas Slebodnik wrote:
>> On (04/04/14 17:47), Dmitri Pal wrote:
>>>>>>> Just a couple minor edits to the language of the doxygen:
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h
>>>>>>> index 9a4eb58..4ef98dc 100644
>>>>>>> - --- a/ini/ini_configobj.h
>>>>>>> +++ b/ini/ini_configobj.h
>>>>>>> @@ -483,6 +483,31 @@ int ini_config_file_open(const char
*filename,
>>>>>>> struct ini_cfgfile **file_ctx);
>>>>>>>
>>>>>>> /**
>>>>>>> + * @brief Create a configuration file object using memory
buffer.
>>>>>>>
>>>>>>> @brief Create a configuration file object from a
NULL-terminated
>>>>>>> buffer.
>>>>>>>
>>>>>>> + *
>>>>>>> + * Create a file object for parsing a configuration file.
>>>>>>> + * Configuration will use provided memory instead of the
actual
>>>>>>> file.
>>>>>>>
>>>>>>> Configuration will use the provided memory buffer instead of
a
>>>>>>> real file.
>>>>>>>
>>>>>>> + *
>>>>>>> + * A "configuration file object" is different
from
>>>>>>> + * a "configuration object". The former stores
metadata
>>>>>>> + * about a file the configuration data is read from,
>>>>>>>
>>>>>>> about a file from which the configuration data is read,
>>>>>>> (this is a very esoteric bit of English)
>>>>>>>
>>>>>>> + * while the latter holds the configuration itself.
>>>>>>>
>>>>>>> while the latter holds the configuration data itself.
>>> I split it into a separate ticket.
>>>
https://fedorahosted.org/sssd/ticket/2307
>>> I suspect that there are other places like this that require some
>>> polish.
>>> Somone needs to review doxygen comments and ding-libs and make them
>>> use better English.
>>>
>>>
>>>> + *
>>>> + * @param[in] data_buf In memory configuration data.
>>>>
>>>> In-memory
>>>>
>>>> + * Needs to be NULL terminated.
>>>>
>>>> NULL-terminated
>>>>
>>>> + * @param[in] data_len Length of memory data
>>>> + * not including terminating NULL.
>>>> + * @param[out] file_ctx Configuration file object.
>>>> + *
>>>> + * @return 0 - Success.
>>>> + * @return EINVAL - Invalid parameter.
>>>> + * @return ENOMEM - No memory.
>>>> + */
>>>> +int ini_config_file_from_mem(void *data_buf,
>>>> + uint32_t data_len,
>>>> + struct ini_cfgfile **file_ctx);
>>>> +
>>>>
>>>>
>>>>
>>>>
>>>> One additional question: is it expected that data_buf will contain NUL
>>>> values in it other than the terminator? If not, why does data_len
>>>> exist?
>>> Interesting question.
>>> Here is how I see it.
>>>
>>> Man page for fmemopen says:
>>>
>>> FILE *fmemopen(void *buf, size_t size, const char *mode);
>>>
>>> ...
>>>
>>> DESCRIPTION
>>> The fmemopen() function opens a stream that permits the
>>> access speci-
>>> fied by mode. The stream allows I/O to be performed on the
>>> string or
>>> memory buffer pointed to by buf. This buffer must be at
>>> least size
>>> bytes long.
>>>
>>>
>>> At least!
>>> But in reality if it is of the same value the function does not work.
>>> I checked it twice and valgrind confirmed that it is trying to read
>>> past the end of the buffer.
>>>
>>> So the NULL termination is the attempt to work around the bug in
>>> fmemopen.
>>> The caller of the function needs to make sure that the function would
>>> work thus we say to NULL terminate the buffer.
>>>
>>> I think I need to add another test. This test would use part of the
>>> buffer not the whole one.
>>>
https://fedorahosted.org/sssd/ticket/2306
>>>
>>>
>>> Can we push the patches as the thread gets long and hard to deal with.
>>> All comments are either addressed or tracked as separate tickets.
>>>
>> I am fine with current version but I have another idea.
>> (I know it is little bit late but it is better to ask before pushing
>> patches to
>> public repo)
>>
>> Could we make next function internal:
>> int ini_config_file_from_mem(void *data_buf,
>> uint32_t data_len,
>> struct ini_cfgfile **file_ctx);
>>
>> and public function will be
>> int ini_config_file_from_string(const char *string_config
>> struct ini_cfgfile **file_ctx);
>>
>> I do not know requirements from yelley; so my idea needn't be
>> acceptable.
>>
>> LS
> I prefer not to for a reason. This function would require to call
> strlen() to find the end of the buffer.
> We do not know anything about the caller or how he allocated buffer so
> we can't call strnlen() which is safer.
> The caller however knows the size of the buffer so it is easier for it
> to get the length in as safe way.
... in _a_ safe way ...
> In many case when you read data from a socket of file you already know
> the size of the data so you do not need to count bytes and thus can
> save cycles.
>
I need to be able to use an array of bytes (containing arbitrary binary data) to
initialize the ini_config object. Since this is binary data, the
ini_config_file_from_string function would not be adequate to handle my use case (since
the NUL character doesn't signify termination for an arbitrary array of bytes).
Regards,
Yassir.