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