Re: [libvirt] [PATCH 2/2] Introduce safewrite_str
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...