Thank you for your comments!
于 2015年07月13日 19:11, Cyril Hrubis 写道:
> Hi!
>> +#ifndef MNT_DETACH
>> +#define MNT_DETACH 2
>> +#endif
>> +
>> +#ifndef MNT_EXPIRE
>> +#define MNT_EXPIRE 4
>> +#endif
>> +
>> +#ifndef UMOUNT_NOFOLLOW
>> +#define UMOUNT_NOFOLLOW 8
>> +#endif
>
> We may add these to allready exsting lapi/mount.h.
>
OK, I got it.
>> +static int fd;
>> +static int fd_new;
>> +static int mount_flag;
>> +
>> +static char buf[256];
>> +
>> +static const char *device;
>> +static const char *fs_type;
>> +static const char *str = "abcdefghijklmnopqrstuvwxyz";
>
> The buf and str are used only in the verify function, why don't decleare
> them as local variables there?
>
OK, I will declare them in verify function.
>> +int main(int ac, char **av)
>> +{
>> + int lc;
>> +
>> + tst_parse_opts(ac, av, NULL, NULL);
>> +
>> + setup();
>> +
>> + for (lc = 0; TEST_LOOPING(lc); lc++) {
>> + tst_count = 0;
>> +
>> + umount2_verify();
>> + }
>> +
>> + cleanup();
>> + tst_exit();
>> +}
>> +
>> +static void setup(void)
>> +{
>> + tst_require_root(NULL);
>> +
>> + tst_sig(NOFORK, DEF_HANDLER, NULL);
>> +
>> + tst_tmpdir();
>> +
>> + fs_type = tst_dev_fs_type();
>> + device = tst_acquire_device(cleanup);
>> +
>> + if (!device)
>> + tst_brkm(TCONF, cleanup, "Failed to obtain block device");
>> +
>> + tst_mkfs(cleanup, device, fs_type, NULL);
>> +
>> + SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
>> +
>> + TEST_PAUSE;
>> +}
>> +
>> +static void umount2_verify(void)
>> +{
>> + SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> + mount_flag = 1;
>> +
>> + fd = SAFE_CREAT(cleanup, MNTPOINT "/file", FILE_MODE);
>> +
>> + do {
>> + TEST(umount2(MNTPOINT, MNT_DETACH));
>> +
>> + if (TEST_RETURN != 0) {
>> + tst_resm(TFAIL, "umount2(2) Failed while "
>> + "unmounting %s errno = %d : %s",
>> + MNTPOINT, TEST_ERRNO,
>> + strerror(TEST_ERRNO));
>
> Use TFAIL | TTERRNO instead of printing
> the TEST_ERRNO yourself.
>
OK,I got it.
>> + break;
>> + }
>> +
>> + mount_flag = 0;
>> +
>> + /* check the unavailability for new access */
>> + fd_new = open(MNTPOINT "/file", O_RDONLY);
>
> What about access() with F_OK instead?
>
Yes, access() is better indeed.
>> + if (fd_new != -1) {
>> + tst_resm(TFAIL, "umount2(2) MNT_DETACH flag "
>> + "performed abnormally "
>> + "expected error = %d : %s",
>> + ENOENT, strerror(ENOENT));
>
> Eh, this is misleading, the errno may
> NOT be ENOENT here.
>
I see, I will remove the "expected error".
>> + break;
>> + }
>> +
>> + /*
>> + * check the old fd still points to the file
>> + * in previous mount point and is available
>> + */
>> + SAFE_WRITE(cleanup, 0, fd, str, strlen(str));
>
> The second parameter should be set to 1 here as we want to assert that
> the whole buffer was written. And the same for the SAFE_READ() below.
>
For SAFE_READ() ,OK I understand that the parameter should be 1 to make
the function more strict in byte verification.
But for SAFE_WRITE() , I can't see there is any difference with the parameter
0 or 1 in safe_write function of lib/safe_macros.c, it is just the strict
verification
no matter the parameter is 0 or 1.
I have sent a patch to fix it, please check that.
>> + SAFE_CLOSE(cleanup, fd);
>> +
>> + SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> + mount_flag = 1;
>> +
>> + fd = SAFE_OPEN(cleanup, MNTPOINT "/file", O_RDONLY);
>> +
>> + SAFE_READ(cleanup, 0, fd, buf, 255);
>> +
>> + if (strcmp(str, buf)) {
>> + tst_resm(TFAIL, "umount2(2) MNT_DETACH flag "
>> + "performed abnormally");
>> + break;
>> + }
>> +
>> + tst_resm(TPASS, "umount2(2) Passed");
>> + } while (0);
>
> The do { } while (0); here is ugly as it adds unceccessary level of
> indentation. Either use tst_brkm() instead of tst_resm() or use the
> kernel style goto (have a look at Linux kernel CodingStyle if you are
> not familiar with that).
>
OK, I got it and going to use the kernel style goto instead.
>> + if (fd_new > 0) {
>> + SAFE_CLOSE(cleanup, fd_new);
>> + fd_new = 0;
>> + }
>> +
>> + SAFE_CLOSE(cleanup, fd);
>> + fd = 0;
>> +
>> + if (mount_flag) {
>> + SAFE_UMOUNT(cleanup, MNTPOINT);
>> + mount_flag = 0;
>> + }
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> + if (fd_new > 0 && close(fd_new))
>> + tst_resm(TWARN | TERRNO, "Failed to close file");
>> +
>> + if (fd > 0 && close(fd))
>> + tst_resm(TWARN | TERRNO, "Failed to close file");
>> +
>> + if (mount_flag && tst_umount(MNTPOINT))
>> + tst_resm(TWARN | TERRNO, "Failed to unmount");
>> +
>> + if (device)
>> + tst_release_device(NULL, device);
>> +
>> + tst_rmdir();
>> +}
>> --
>> 1.8.4.2
>>
>>
>> ------------------------------------------------------------------------------
>> Don't Limit Your Business. Reach for the Cloud.
>> GigeNET's Cloud Solutions provide you with the tools and support that
>> you need to offload your IT needs and focus on growing your business.
>> Configured For All Businesses. Start Your Cloud Today.
>> https://www.gigenetcloud.com/
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>
.
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
