On 05/23/14 19:29, Eric Blake wrote:
> On 05/22/2014 07:47 AM, Peter Krempa wrote:
>> Stat the path of the storage file being tested to set the correct type
>> into the virStorageSource. This will avoid breaking the test suite when
>> inquiring metadata of directory paths in the next patches.
>> ---
>> tests/virstoragetest.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
>> index fb96c71..746bf63 100644
>> --- a/tests/virstoragetest.c
>> +++ b/tests/virstoragetest.c
>> @@ -98,6 +98,7 @@ testStorageFileGetMetadata(const char *path,
>> uid_t uid, gid_t gid,
>> bool allow_probe)
>> {
>> + struct stat st;
>> virStorageSourcePtr ret = NULL;
>>
>> if (VIR_ALLOC(ret) < 0)
>> @@ -106,6 +107,15 @@ testStorageFileGetMetadata(const char *path,
>> ret->type = VIR_STORAGE_TYPE_FILE;
>> ret->format = format;
>>
>> + if (stat(path, &st) == 0) {
>
> Same question as in 12/33 on whether we should cache a struct stat to
Not in this case. This code here is used to set the correct type of the
file prior to the first iteration of the loop in the tests. The test
code doesn't initialize the type of the volume to the actual type (for
directories) so we need to fix it up before iterating. Otherwise the
test would fail in the later patches as it would try to access a
directory as a file.
> minimize the calls to stat() (or at most have one stat() prior to open,
> and one fstat() after open or after any resize operation). Another good
> reason to minimize stats, as well as to trust open/fstat more than
> stat/open, is to minimize TOCTTOU races. But this patch makes sense
This argument would be relevant in the previous patch. Here in the test
code there is no possible race.
> whether or not we do that as a followup.
>
> ACK.
>
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list