On Tue, Sep 12, 2017 at 03:05:43PM +0200, Pino Toscano wrote:
On Tuesday, 12 September 2017 14:29:13 CEST Richard W.M. Jones
wrote:
> +/**
> + * Generic functions for reading and writing the cache files, used
> + * where we are just reading and writing plain text strings.
> + */
> +static int
> +generic_read_cache (guestfs_h *g, const char *filename, char **strp)
> +{
> + if (access (filename, R_OK) == -1 && errno == ENOENT)
> + return 0; /* no cache, run the test instead */
This will go ahead if access() failed for any other error though;
IMHO a better check could be:
if (access (filename, R_OK) == -1) {
if (errno == ENOENT)
return 0; /* no cache, run the test instead */
perrorf (g, "access: %s", filename);
return -1;
}
But isn't it OK since the following line will return the true error:
if (guestfs_int_read_whole_file (g, filename, strp, NULL) == -1)
...
?
> +static int
> +generic_write_cache (guestfs_h *g, const char *filename, const char *str)
> +{
> + CLEANUP_FCLOSE FILE *fp = fopen (filename, "w");
> + if (fp == NULL) {
> + perrorf (g, "%s", filename);
> + return -1;
> + }
> +
> + if (fprintf (fp, "%s", str) == -1) {
> + perrorf (g, "%s: write", filename);
> + return -1;
> + }
> +
> + return 0;
> +}
While this is the same code already used, IMHO it would make more sense
to use open/write directly (since we have a buffer already, it will be
faster than using sprintf); there are snippets that call write() in a
loop until the whole buffer is written in different parts of the
library, so factorizing them could help.
OK (using full_write).
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top