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

[Pavel Emelyanov]

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.

> this looks like overdone for me.
> 
>> I think we should have one method for one type of problems.
>> I agree with both of them, so you should chage it everywhere.
> 
> 	Cyrill
> .
>