----- Original Message ----- > 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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
