[PATCH] xfstests: fix up fs_perms test used by 126

[PATCH] xfstests: fix up fs_perms test used by 126

[Eric Sandeen]

Test 126 was failing intermittently for Ted & I; it seems that
this is because we were passing an unterminated string to
fopen for the mode; I'm not certain why this made it fail,
but it's pretty clearly not a good thing to do, and fixing
it fixes the test.

Rather than passing around characters, do things string-wise,
since that is what is ultimately used in fopen().

Also make it at least possible to pass in the 2-character
modes fopen can take (r+, w+, etc), I suppose testcases
could be added for this later.

Reported-by: Theodore Tso <tytso@mit.edu>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

diff --git a/src/fs_perms.c b/src/fs_perms.c
index 2c5e3fa..f34c4f4 100644
--- a/src/fs_perms.c
+++ b/src/fs_perms.c
@@ -42,7 +42,7 @@ int testsetup(mode_t mode, int cuserId, int cgroupId);
 int testfperm(int userId, int groupId, char* fperm);
 
 int main( int argc, char *argv[]) {
-   char fperm[1];
+   char fperm[3];
    int result, exresult=0,  cuserId=0, cgroupId=0, userId=0, groupId=0;
    mode_t mode;
 
@@ -53,7 +53,8 @@ int main( int argc, char *argv[]) {
               cgroupId = atoi(argv[3]);
               userId = atoi(argv[4]);
               groupId = atoi(argv[5]);
-              fperm[0] = *argv[6];
+              strncpy(fperm, argv[6], 3);
+              fperm[2] = '\0';
               exresult = atoi(argv[7]);
 	      break;
       default:
@@ -64,7 +65,7 @@ int main( int argc, char *argv[]) {
    testsetup(mode,cuserId,cgroupId);
    result=testfperm(userId,groupId,fperm);
    system("rm test.file");
-   printf("%c a %03o file owned by (%d/%d) as user/group(%d/%d)  ",fperm[0],mode,cuserId,cgroupId,userId,groupId);
+   printf("%s a %03o file owned by (%d/%d) as user/group(%d/%d)  ",fperm,mode,cuserId,cgroupId,userId,groupId);
    if (result == exresult) {
       printf("PASS\n");
       exit(0);
@@ -102,8 +103,7 @@ int testfperm(int userId, int groupId, char* fperm) {
            return(-1);
         }
 
-    switch(tolower(fperm[0])) {
-       case 'x': 
+    if (!strcmp("x", fperm)) {
           PID = fork();
 	  if (PID == 0) {
              execlp("./test.file","test.file",NULL); 
@@ -114,8 +114,7 @@ int testfperm(int userId, int groupId, char* fperm) {
           seteuid(0);
           setegid(0);
 	  return(nuthertmpi);
-
-       default: 
+    } else {
           if((testfile=fopen("test.file",fperm))){
             fclose(testfile);
             seteuid(0);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: fix up fs_perms test used by 126

[Christoph Hellwig]

On Fri, Feb 05, 2010 at 10:59:45AM -0600, Eric Sandeen wrote:
> @@ -53,7 +53,8 @@ int main( int argc, char *argv[]) {
>                cgroupId = atoi(argv[3]);
>                userId = atoi(argv[4]);
>                groupId = atoi(argv[5]);
> -              fperm[0] = *argv[6];
> +              strncpy(fperm, argv[6], 3);
> +              fperm[2] = '\0';

This still looks rather weird to me.  What's the reason for copying
the string into a fixed length buffer?  Why not leave fperm as a pointer
to the original argument?

The rest of the patch looks fine, but a clean up pass on the whole
file wouldn't hurt either, it's a grotty mess..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: fix up fs_perms test used by 126

[Eric Sandeen]

Christoph Hellwig wrote:
> On Fri, Feb 05, 2010 at 10:59:45AM -0600, Eric Sandeen wrote:
>> @@ -53,7 +53,8 @@ int main( int argc, char *argv[]) {
>>                cgroupId = atoi(argv[3]);
>>                userId = atoi(argv[4]);
>>                groupId = atoi(argv[5]);
>> -              fperm[0] = *argv[6];
>> +              strncpy(fperm, argv[6], 3);
>> +              fperm[2] = '\0';
> 
> This still looks rather weird to me.  What's the reason for copying
> the string into a fixed length buffer?  Why not leave fperm as a pointer
> to the original argument?

eh that's probably better, I guess I was just thinking copy
based on how it was before.  (which copied the char, right, it
didn't assign a pointer, unless I'm short on coffee today...)

OTOH fopen only takes 2 chars anyway.  But probably no reason to
truncate what was given, just fail if it's something that's wrong...

-Eric
 
> The rest of the patch looks fine, but a clean up pass on the whole
> file wouldn't hurt either, it's a grotty mess..
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V2] xfstests: fix up fs_perms test used by 126

[Eric Sandeen]

Test 126 was failing intermittently for Ted & I; it seems that
this is because we were passing an unterminated string to
fopen for the mode; I'm not certain why this made it fail,
but it's pretty clearly not a good thing to do, and fixing
it fixes the test.

Rather than passing around characters, do things string-wise,
since that is what is ultimately used in fopen().

Reported-by: Theodore Tso <tytso@mit.edu>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

V2: Just pass around pointer to passed-in mode argument

diff --git a/src/fs_perms.c b/src/fs_perms.c
index 2c5e3fa..ea188c4 100644
--- a/src/fs_perms.c
+++ b/src/fs_perms.c
@@ -42,7 +42,7 @@ int testsetup(mode_t mode, int cuserId, int cgroupId);
 int testfperm(int userId, int groupId, char* fperm);
 
 int main( int argc, char *argv[]) {
-   char fperm[1];
+   char *fperm;
    int result, exresult=0,  cuserId=0, cgroupId=0, userId=0, groupId=0;
    mode_t mode;
 
@@ -53,7 +53,7 @@ int main( int argc, char *argv[]) {
               cgroupId = atoi(argv[3]);
               userId = atoi(argv[4]);
               groupId = atoi(argv[5]);
-              fperm[0] = *argv[6];
+              fperm = argv[6];
               exresult = atoi(argv[7]);
 	      break;
       default:
@@ -64,7 +64,7 @@ int main( int argc, char *argv[]) {
    testsetup(mode,cuserId,cgroupId);
    result=testfperm(userId,groupId,fperm);
    system("rm test.file");
-   printf("%c a %03o file owned by (%d/%d) as user/group(%d/%d)  ",fperm[0],mode,cuserId,cgroupId,userId,groupId);
+   printf("%s a %03o file owned by (%d/%d) as user/group(%d/%d)  ",fperm,mode,cuserId,cgroupId,userId,groupId);
    if (result == exresult) {
       printf("PASS\n");
       exit(0);
@@ -102,8 +102,7 @@ int testfperm(int userId, int groupId, char* fperm) {
            return(-1);
         }
 
-    switch(tolower(fperm[0])) {
-       case 'x': 
+    if (!strcmp("x", fperm)) {
           PID = fork();
 	  if (PID == 0) {
              execlp("./test.file","test.file",NULL); 
@@ -114,8 +113,7 @@ int testfperm(int userId, int groupId, char* fperm) {
           seteuid(0);
           setegid(0);
 	  return(nuthertmpi);
-
-       default: 
+    } else {
           if((testfile=fopen("test.file",fperm))){
             fclose(testfile);
             seteuid(0);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V2] xfstests: fix up fs_perms test used by 126

[Christoph Hellwig]

On Tue, Feb 09, 2010 at 10:54:54AM -0600, Eric Sandeen wrote:
> Test 126 was failing intermittently for Ted & I; it seems that
> this is because we were passing an unterminated string to
> fopen for the mode; I'm not certain why this made it fail,
> but it's pretty clearly not a good thing to do, and fixing
> it fixes the test.
> 
> Rather than passing around characters, do things string-wise,
> since that is what is ultimately used in fopen().
> 
> Reported-by: Theodore Tso <tytso@mit.edu>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs