You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/14 06:44:34 UTC

[GitHub] [incubator-nuttx-apps] VanFeo opened a new pull request #1063: testing/fstest: add cleanup to fatutf8

VanFeo opened a new pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063


   N/A
   Signed-off-by: fanzhuyun <fa...@xiaomi.com>
   
   Change-Id: If458048804daced1bcc8f1ee82a5e2a16bf6140b
   
   ## Summary
   
   ## Impact
   
   ## Testing
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] VanFeo commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
VanFeo commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r826560864



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -166,5 +168,28 @@ int main(int argc, FAR char *argv[])
       exit(fd);
     }
 
+  printf("\n");
+
+  ret = remove(path);
+  if (ret == 0)
+    {
+      printf("removed %s\n", path);
+      ret = remove(dir_path);

Review comment:
       It is a good idea and has been revised




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r826790524



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -108,6 +109,7 @@ int main(int argc, FAR char *argv[])
 
   printf("\n");
 
+  strcpy(dir_path, path);

Review comment:
       It's impossible since https://github.com/apache/incubator-nuttx-apps/pull/1063/files#diff-c6c45b54fda3ab97655d3becbfe72f45338ed829e17a019b868a6999e62ceb6cR98-R103 report EEXIST.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r826817735



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -108,6 +109,7 @@ int main(int argc, FAR char *argv[])
 
   printf("\n");
 
+  strcpy(dir_path, path);

Review comment:
       Ok




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r826566231



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -52,6 +52,7 @@ static void show_usage(FAR const char *progname, int exitcode)
 #define BASEPATH "/mnt/sdcard"
 #define FOLDER_NAME "/ÄÖÜ 测试文件夹 Test Folder"
 #define FILE_NAME "/ÄÖÜ 测试文件 Test File"
+#define PATH_MAX 256

Review comment:
       why define PATH_MAX which is a standard macro?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] VanFeo commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
VanFeo commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r826587559



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -52,6 +52,7 @@ static void show_usage(FAR const char *progname, int exitcode)
 #define BASEPATH "/mnt/sdcard"
 #define FOLDER_NAME "/ÄÖÜ 测试文件夹 Test Folder"
 #define FILE_NAME "/ÄÖÜ 测试文件 Test File"
+#define PATH_MAX 256

Review comment:
       removed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1063: testing/fstest: add cleanup to fatutf8

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1063:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1063#discussion_r825660667



##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -108,6 +109,7 @@ int main(int argc, FAR char *argv[])
 
   printf("\n");
 
+  strcpy(dir_path, path);

Review comment:
       I do not think that removal of a directory unconditionally is a good idea. What if directory existed already and was intentionally created by user? The removal in such case will be confusing. Can we better check if directory is already created and delete only if it was created by this code? 

##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -166,5 +168,28 @@ int main(int argc, FAR char *argv[])
       exit(fd);
     }
 
+  printf("\n");
+
+  ret = remove(path);
+  if (ret == 0)
+    {
+      printf("removed %s\n", path);
+      ret = remove(dir_path);

Review comment:
       Maybe we can use next code instead of adding new variable to stack?
   ```
   basepath = strrchr(path, '/');
   if (basepath != NULL)
     {
       *basepath = '\0';
       ret = remove(path);
       if (ret == 0)
   ...
     }
   ```

##########
File path: testing/fatutf8/fatutf8_main.c
##########
@@ -61,6 +61,7 @@ int main(int argc, FAR char *argv[])
 
   char buf[128];
   char path[256];
+  char dir_path[256];

Review comment:
       ```suggestion
     char path[PATH_MAX];
     char dir_path[PATH_MAX];
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org