yamt commented on code in PR #15326:
URL: https://github.com/apache/nuttx/pull/15326#discussion_r1897172333


##########
libs/libc/misc/lib_tempbuffer.c:
##########
@@ -63,58 +63,61 @@ static struct pathbuffer_s g_pathbuffer =
  ****************************************************************************/
 
 /****************************************************************************
- * Name: lib_get_pathbuffer
+ * Name: lib_get_tempbuffer
  *
  * Description:
- *   The lib_get_pathbuffer() function returns a pointer to a temporary
+ *   The lib_get_tempbuffer() function returns a pointer to a temporary
  *   buffer.  The buffer is allocated from a pool of pre-allocated buffers
  *   and if the pool is exhausted, a new buffer is allocated through
- *   kmm_malloc(). The size of the buffer is PATH_MAX, and must freed by
- *   calling lib_put_pathbuffer().
+ *   kmm_malloc(). The size of the buffer is nbytes, and must freed by
+ *   calling lib_put_tempbuffer().
  *
  * Returned Value:
- *   On success, lib_get_pathbuffer() returns a pointer to a temporary
+ *   On success, lib_get_tempbuffer() returns a pointer to a temporary
  *   buffer.  On failure, NULL is returned.
  *
  ****************************************************************************/
 
-FAR char *lib_get_pathbuffer(void)
+FAR char *lib_get_tempbuffer(size_t nbytes)
 {
-  for (; ; )
+  if (nbytes <= TEMP_MAX_SIZE)

Review Comment:
   i'm not sure how it's different from malloc either.
   
   > Allocate from heap, which may increase memory fragment and impact 
performance
   
   do you mean line buffer allocation in nsh is performance critical? really?
   also, as far as the allocation lifetime is very short, fragmentation is not 
a big concern i suspect.
   



##########
fs/littlefs/lfs_vfs.c:
##########
@@ -1531,7 +1531,7 @@ static int littlefs_mkdir(FAR struct inode *mountpt, FAR 
const char *relpath,
 
   if (len > 0 && relpath[len - 1] == '/')
     {
-      path = lib_get_pathbuffer();
+      path = lib_get_tempbuffer(PATH_MAX);

Review Comment:
   i feel it's simpler to make
   ```
   #define lib_get_pathbuffer() lib_get_tempbuffer(PATH_MAX)
   ```
   instead of modifying all callers.
   



##########
sched/Kconfig:
##########
@@ -1412,6 +1412,13 @@ config PATH_MAX
        ---help---
                The maximum size of path name.
 
+config LINE_MAX
+       int "Maximum size of line"
+       default 64 if DEFAULT_SMALL
+       default 80 if !DEFAULT_SMALL
+       ---help---
+               The maximum size of line.

Review Comment:
   please add explanation about what "line" here is.



-- 
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-unsubscr...@nuttx.apache.org

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

Reply via email to