[CRIU] [PATCH 1/8] util: Add read_string helper

[Cyrill Gorcunov]

On Thu, May 10, 2012 at 02:17:58PM +0400, Pavel Emelyanov wrote:
> On 05/10/2012 11:21 AM, Cyrill Gorcunov wrote:
> > On Thu, May 10, 2012 at 11:04:52AM +0400, Andrew Vagin wrote:
> >> On Sat, May 05, 2012 at 07:20:17PM +0400, Cyrill Gorcunov wrote:
> >>>
> >>> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> >>> ---
> >>>  include/util.h |    7 +++++++
> >>>  1 files changed, 7 insertions(+), 0 deletions(-)
> >>>
> >>
> >>> diff --git a/include/util.h b/include/util.h
> >>> index 5172a88..62e3877 100644
> >>> --- a/include/util.h
> >>> +++ b/include/util.h
> >>> @@ -132,6 +132,13 @@ static inline int read_img_buf(int fd, void *ptr, int size)
> >>>  #define memzero_p(p)		memset(p, 0, sizeof(*p))
> >>>  #define memzero(p, size)	memset(p, 0, size)
> >>>  
> >>> +static inline int read_string(int fd, char *ptr, int size)
> >>> +{
> >>> +	int ret = read(fd, ptr, size);
> >>> +	ptr[ret > 0 ? ret - 1 : 0] = '\0';
> > 
> >> I don't like it, the last symbol may be non-zero, but we set it to zero,
> >> it may hide a problem.
> > 
> > This is done by design. Sure we can return -ENOMEM in case if string
> > is trimmed, still it must be guaranted that returned result _is_
> > as C-string as function implies.
> > 
> >> 	if (ret < 0)
> >> 		return ret;
> >> 	else if (ret == size) {
> >> 		if (ptr[ret - 1] != '\0')
> >> 			return -ENOMEM;
> >> 		return ret;
> >> 	} else
> >> 		ptr[ret] = '\0';
> >>
> >> Why can you not use my decision for similar cases?
> > 
> > I don't like the idea of introducing global variables, this
> 
> You can declare Andrey's buffers locally.
> 
> > kills ability to use functions in parallel execution. Another
> > misdesign I think is that instead of implementing safe-string-read
> > in a function itself we introduce 2 requirements
> > 
> >  - a _speacially_ configured buffer must be passed
> 
> Exactly.
> 
> >  - a reader must take into account that buffer has zero byte
> >    at the end
> 
> It's always OK.
> 
> Andrey's buffers are cool, because you can use them with any stream reading
> function (read, fgets, whatever else). Your approach requires writing a helper
> for each.

fgets does exatly what we need -- it writes EOS at the end of the buffer,
you dont need to pass special buffer for it.

The benefit of using EOS based buffer is that you don't need to setup \0
every call the buffer used, it's written once for all life-time of buffer
(which I think improves speed in case of reading data in a loop but this
 is microoptimization).

I'll think how to use Andrew's biffers here.

	Cyrill