[libvirt] [PATCH 2/2] Introduce safewrite_str

[Ján Tomko]

On Thu, Feb 18, 2016 at 07:40:12AM -0500, John Ferlan wrote:
> 
> 
> On 02/12/2016 08:59 AM, Ján Tomko wrote:
> > Just like safewrite, but calls strlen first to figure out
> > the length of the string.
> > ---
> >  src/conf/virchrdev.c       |  2 +-
> >  src/libvirt_private.syms   |  1 +
> >  src/lxc/lxc_process.c      |  4 ++--
> >  src/network/leaseshelper.c |  2 +-
> >  src/openvz/openvz_conf.c   | 15 ++++++---------
> >  src/qemu/qemu_domain.c     |  2 +-
> >  src/util/vircommand.c      |  4 ++--
> >  src/util/virfile.c         |  9 ++++++++-
> >  src/util/virfile.h         |  1 +
> >  src/util/virlog.c          |  6 +++---
> >  src/util/virpidfile.c      |  4 ++--
> >  src/util/virxml.c          | 17 ++++++-----------
> >  tools/vsh.c                |  4 +---
> >  13 files changed, 35 insertions(+), 36 deletions(-)
> > 
> 
> Conflicted about this one - the difference between the two appears to be
> 'safewrite' will write a some length of a string (e.g. a counted length)
> while safewrite_str writes the entire string.

Safewrite writes data from a buffer, it does not have to be a string
(i.e. it can write more than a string).

> To make things more
> interesting some safewrite calls use sizeof instead of strlen, but in
> reality are just writing everything in the buffer.
> 
> So, perhaps 'safewrite_all' or 'safewrite_full' better describes the new
> functionality?
> 

Unlike virFileReadAll where we get EOF after 'all' ends, there is no
such marker when reading from memory. We have no way of telling what
'all' is unless we opreate on a string.

> I also started looking at some of the other safewrite calls to see if
> any more fell into the bucket of write everything and few more jumped
> out at me. Some used sizeof instead of strlen and some uses of strlen
> were better hidden, such as:
> 
>   * secretSaveDef vectors through replaceFile

That whole function reimplements virFileRewrite.

>   * update_include_file
> 

And this one could just reuse the return value of virAsprintf.

> The other thing that I noted is error checking is not consistent. While
> checking < 0 does ensure that the write() call didn't fail, what if the
> number of bytes written != the number of bytes we tried to write?
> 

Our safewrite function calls write in a loop, so this would only happen
if write returns success and returned zero bytes on the last call, which
should not ever happen (TM) for regular files.

Jan

> So maybe this is a good opportunity to make all this consistent for all
> callers.
> 
> BTW: saferead is in a similar situation...
>