qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Marcel Apfelbaum <marcel.a@redhat.com>
To: Michael Tokarev <mjt@tls.msk.ru>
Cc: qemu-trivial@nongnu.org, Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] acpi unit-test: Create temporary disk file under /tmp
Date: Fri, 20 Dec 2013 20:27:30 +0200	[thread overview]
Message-ID: <1387564050.28388.9.camel@localhost.localdomain> (raw)
In-Reply-To: <52B401A0.2010403@msgid.tls.msk.ru>

On Fri, 2013-12-20 at 12:36 +0400, Michael Tokarev wrote:
> 20.12.2013 11:05, Fam Zheng wrote:
> > As other tests, the image file is created in /tmp other than current
> > dir. Thus there will not be an unignored file under tests for intree
> > build.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  tests/acpi-test.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > index ca83b1d6..e4f31b7 100644
> > --- a/tests/acpi-test.c
> > +++ b/tests/acpi-test.c
> > @@ -116,7 +116,7 @@ static uint8_t boot_sector[0x200] = {
> >      [0x1FF] = 0xAA,
> >  };
> >  
> > -static const char *disk = "tests/acpi-test-disk.raw";
> > +static const char *disk = "/tmp/qtest-acpi-test-disk.raw";
> 
> Please, PLEASE do NOT do this.
> 
> If you want a temp dir, create one in qemu source directory
> (there's no urge to be able to specify one at build/run time,
> a symlink created by user will do), -- so that all this junk
> will be in a single dir easy to remove.
> 
> But never, ever, use /tmp like this, not here not elsewhere
> else.  When you use /tmp, first, there's $TEMPDIR, and second,
> much more important, there are apis like mkstemp(3) or
> tmpfile(3), or tempnam(3), -- THIS is the only proper way to
> use common temporary directories.
> 
> Never, ever, create fixed (or even predictable, with getpid()
> for example) files in public temporary dirs.  Never.
> 
> For this issue, I think a better solution is to actually remove
> this file on make clean.  Now you're just moving an unignored
> file elsewhere, which isn't good by its own.

I would use (and let glib do the work): 
    fd = g_file_open_tmp("prefix-XXXXXX", file_name, &error);
    g_assert_no_error(error);
    ...
and of course in the end:
    unlink(file_name);

I hope I helped,
Marcel


> 
> Thanks,
> 
> /mjt

      parent reply	other threads:[~2013-12-20 18:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-20  7:05 [Qemu-devel] [PATCH] acpi unit-test: Create temporary disk file under /tmp Fam Zheng
2013-12-20  7:36 ` Paolo Bonzini
2013-12-20  8:36 ` Michael Tokarev
2013-12-20  9:29   ` Fam Zheng
2013-12-20 18:27   ` Marcel Apfelbaum [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387564050.28388.9.camel@localhost.localdomain \
    --to=marcel.a@redhat.com \
    --cc=famz@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).