Re: r1358110 - Promote two identical readline() helper functions to one

> Author: stsp

> Date: Fri Jul  6 10:45:21 2012
> New Revision: 1358110
>
> URL: http://svn.apache.org/viewvc?rev=1358110&view=rev
> Log:
> Promote two identical readline() helper functions to one svn_io_file_readline()
> public API function.
>
> * subversion/include/svn_io.h
>   (svn_io_file_readline): Declare.
[...]

Hi stefan.

+1 on combining two duplicate copies into one...

> Modified: subversion/trunk/subversion/include/svn_io.h
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Fri Jul  6 10:45:21 2012
>
> +/* Read a line of text from a file, up to a specified length.

This is much like a variant of svn_io_read_length_line().  Can we move its declaration and definition adjacent to that function, and cross-reference their doc strings?

Can they share implementation?  They look similar.  svn_io_read_length_line() was optimized in r1381800, but this function was not.

This is also a bit like svn_stream_readline().  It has in common with svn_stream_readline() that the result is returned in a stringbuf.  However, svn_io_read_length_line() writes into a plain memory area.

All three of these functions differ in how they handle EOLs.

The different combinations of output style and EOL handling tend to make the overall API get confusing.  Can we do something to reduce that amount of difference?

One simple short-term solution to all the above would be to just make this new API private for the time being.

- Julian

> + * Allocate @a *stringbuf in @a result_pool, and read into it one line
> + * from @a file. Reading stops either after a line-terminator was found
> + * or after @a max_len bytes have been read.
> + *
> + * If end-of-file is reached or @a max_len bytes have been read, and @a eof
> + * is not NULL, then set @a *eof to @c TRUE.

Confusing that *eof can be set when it's not end-of-file.  Can we set *eof only when it's end-of-file?

> + * The line-terminator is not stored in @a *stringbuf.
> + * The line-terminator is detected automatically and stored in @a *eol
> + * if @a eol is not NULL. If EOF is reached and @a file does not end
> + * with a newline character, and @a eol is not NULL, @ *eol is set to NULL.
> + *
> + * @a scratch_pool is used for temporary allocations.
> + *
> + * Hint: To read all data until a line-terminator is hit, pass APR_SIZE_MAX
> + * for @a max_len.
> + *
> + * @since New in 1.8.
> + */
> +svn_error_t *
> +svn_io_file_readline(apr_file_t *file,
> +                    svn_stringbuf_t **stringbuf,
> +                    const char **eol,
> +                    svn_boolean_t *eof,
> +                    apr_size_t max_len,
> +                    apr_pool_t *result_pool,
> +                    apr_pool_t *scratch_pool);
Received on 2012-09-19 03:35:44 CEST