Re: [PATCH 1/3] Use g_mkstemp_full instead of mkostemp(s)

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Thu, Nov 14, 2019 at 05:37:26PM +0100, Peter Krempa wrote:
> On Thu, Nov 14, 2019 at 14:48:04 +0100, Ján Tomko wrote:
> > With g_mkstemp_full, there is no need to distinguish between
> > mkostemp and mkostemps (no suffix vs. a suffix of a fixed length),
> > because the GLib function looks for the XXXXXX pattern everywhere
> > in the string.
> >
> > Use S_IRUSR | S_IWUSR for the permissions and do not pass O_RDWR
> > in flags since it's implied.
> >
> > Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> > ---
> >  src/qemu/qemu_driver.c       | 8 ++++----
> >  src/storage/storage_driver.c | 2 +-
> >  src/storage/storage_util.c   | 2 +-
> >  src/util/virlog.c            | 8 +-------
> >  src/vbox/vbox_common.c       | 4 ++--
> >  tests/virfiletest.c          | 2 +-
> >  tools/vsh.c                  | 4 ++--
> >  7 files changed, 12 insertions(+), 18 deletions(-)
>
> [...]
>
> > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> > index 04e4abcd6a..d8355d3c3c 100644
> > --- a/src/storage/storage_driver.c
> > +++ b/src/storage/storage_driver.c
> > @@ -2825,7 +2825,7 @@ virStoragePoolObjFindPoolByUUID(const unsigned char *uuid)
> >   *
> >   * Generate a name for a temporary file using the driver stateDir
> >   * as a path, the pool name, and the volume name to be used as input
> > - * for a mkostemp
> > + * for mkstemp
>
> Shouldn't we mention g_mkstemp_full?

Well, the XXXXXX template is not really g_mkstemp-specific

> >   *
> >   * Returns a string pointer on success, NULL on failure
> >   */
>
> [...]
>
> > diff --git a/tools/vsh.c b/tools/vsh.c
> > index 000cf6a009..e851303e69 100644
> > --- a/tools/vsh.c
> > +++ b/tools/vsh.c
> > @@ -2400,9 +2400,9 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
> >      tmpdir = getenv("TMPDIR");
> >      if (!tmpdir) tmpdir = "/tmp";
> >      ret = g_strdup_printf("%s/virshXXXXXX.xml", tmpdir);
> > -    fd = mkostemps(ret, 4, O_CLOEXEC);
> > +    fd = g_mkstemp_full(ret, O_CLOEXEC, S_IRUSR | S_IWUSR);
> >      if (fd == -1) {
> > -        vshError(ctl, _("mkostemps: failed to create temporary file: %s"),
> > +        vshError(ctl, _("g_mkstemp: failed to create temporary file: %s"),
>
> _full

Fixed.

Jano

> >                   virStrerror(errno, ebuf, sizeof(ebuf)));
> >          VIR_FREE(ret);
> >          return NULL;
>
> Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list