rename lib_pathbuffer to lib_tempbuffer

[Zhangshoukui]

Summary

rename lib_pathbuffer to lib_tempbuffer

Impact

None

Testing

./tools/configure.sh -l sim:nsh
make -j7

[nuttxpr]

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details.

Here's what's missing:

• Summary: It states what is being changed (renaming lib_pathbuffer to lib_tempbuffer), but not why. What problem does this renaming solve? What was the motivation for this change? How does the renaming work (is it a simple rename, or are there code changes associated with it)? Are there related NuttX issues?
• Impact: Simply stating "None" is insufficient. Even a seemingly simple rename can have unforeseen impacts. The PR needs to explicitly address each impact point with justification. For example, why is there no impact on documentation? If there truly is no impact, explicitly state "NO" for each item.
• Testing: While it mentions build commands, it provides no actual testing logs. Showing the output before and after the change is crucial to demonstrate that the change works as intended and doesn't introduce regressions. What functionality was tested? What were the expected results? Also, specifying only the build host is insufficient; the target architecture (e.g., sim:nsh) should be listed under Target(s) with details about the board and configuration. Furthermore, mentioning the build host operating system, CPU architecture, and compiler version is essential.

In short, the PR needs to be significantly more detailed to meet the NuttX requirements. It needs to explain the rationale behind the change, justify the claimed lack of impact, and provide concrete evidence of testing.

[anchao]

I don't understand this change. If the required path buffer size is large than PATH_MAX, why not just call malloc() in application? Why must use this API?
I haven't seen lib_get_pathbuffer() used anywhere else except the kernel, 100% of the source code is using PATH_MAX, why add this redundant check?

[Zhangshoukui]

1. nbytes <= TEMP_MAX_SIZE: Make sure don't cross the line
2. At present, parameter passing is supported, and the code under the following app uses this interface to optimize the use of the stack. The new feature of nshlib has exceeded the call stack of 2k.

[anchao]

You should call malloc in nshlib instead of making the interface so ugly

[xiaoxiang781216]

many places (especially in nsh) need allocate temp buffer (e.g. path or line) only in function scope, both follow approach isn't perfect:

1. Allocate from stack, which already cause the default 2KB stack is unusable in many cases:
   {bp-15165} rv-virt/citest: Increase init task stack size to 3072 #15173
   esp32s3: Increase the init task stask size when using NSH #14501
2. Allocate from heap, which may increase memory fragment and impact performance

so, it's reasonable to extend pathbuffer to tempbuffer, and let's caller utilize the general buffer facility when they find some function occupy too much stack space.

You should call malloc in nshlib instead of making the interface so ugly

It's natural to add size argument to a buffer allocation function. Do you feel ugly malloc with size or not strange malloc without size?

[anchao]

The implementation of pathbuffer is a memory pool with a limited length. Currently, all the kernel code is using the length of PATH_MAX. From the kernel perspective, add length parameter is a strange design, because the kernel code can ensure that all locations(100%) that call pathbuffer only use PATH_MAX:

FAR char *lib_get_pathbuffer(void);
-> FAR char *lib_get_tempbuffer(size_t nbytes); <- why?

If the customized length in the application exceeds PATH_MAX, we should manage the life cycle of this memory segment by application self. If the stack occupies too much, we can choose to define it statically or call malloc:

https://github.com/apache/nuttx-apps/blob/master/nshlib/nsh_console.h#L157

The design of the API should be more reasonable and easy to use. If 99% of the code is using PATH_MAX as the expected value, why should these codes bear the overhead of parameter passing and parameter checking? This is unreasonable

[xiaoxiang781216]

keep in the soure code

[xiaoxiang781216]

#  define lib_get_tempbuffer(n) alloca(n)

[xiaoxiang781216]

many places (especially in nsh) need allocate temp buffer (e.g. path or line) only in function scope, both follow approach isn't perfect:

1. Allocate from stack, which already cause the default 2KB stack is unusable in many cases:
   {bp-15165} rv-virt/citest: Increase init task stack size to 3072 #15173
   esp32s3: Increase the init task stask size when using NSH #14501
2. Allocate from heap, which may increase memory fragment and impact performance

so, it's reasonable to extend pathbuffer to tempbuffer, and let's caller utilize the general buffer facility when they find some function occupy too much stack space.

You should call malloc in nshlib instead of making the interface so ugly

It's natural to add size argument to a buffer allocation function. Do you feel ugly malloc with size or not strange malloc without size?