* Re: [Qemu-devel] [PATCH] acpi unit-test: Create temporary disk file under /tmp
@ 2013-12-20 8:36 ` Michael Tokarev
0 siblings, 0 replies; 10+ messages in thread
From: Michael Tokarev @ 2013-12-20 8:36 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-trivial, qemu-devel, Marcel Apfelbaum
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.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-trivial] [PATCH] acpi unit-test: Create temporary disk file under /tmp
2013-12-20 8:36 ` [Qemu-devel] " Michael Tokarev
@ 2013-12-20 9:29 ` Fam Zheng
-1 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-12-20 9:29 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel, Marcel Apfelbaum
On 12/20/13 16:36, 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.
>
OK, will take your suggestion. Thanks for explanation.
Fam
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] acpi unit-test: Create temporary disk file under /tmp
@ 2013-12-20 9:29 ` Fam Zheng
0 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-12-20 9:29 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel, Marcel Apfelbaum
On 12/20/13 16:36, 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.
>
OK, will take your suggestion. Thanks for explanation.
Fam
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-trivial] [PATCH] acpi unit-test: Create temporary disk file under /tmp
2013-12-20 8:36 ` [Qemu-devel] " Michael Tokarev
@ 2013-12-20 18:27 ` Marcel Apfelbaum
-1 siblings, 0 replies; 10+ messages in thread
From: Marcel Apfelbaum @ 2013-12-20 18:27 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Fam Zheng, qemu-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] acpi unit-test: Create temporary disk file under /tmp
@ 2013-12-20 18:27 ` Marcel Apfelbaum
0 siblings, 0 replies; 10+ messages in thread
From: Marcel Apfelbaum @ 2013-12-20 18:27 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Fam Zheng, qemu-devel
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
^ permalink raw reply [flat|nested] 10+ messages in thread