[PATCH] libmenu: fill allocated memory for path variable with 0, fixes menu_keepdir
HI all, This patch does $subject. The code is : if (!fstat (path_fp, &st) && (st.st_size > 0)) { path = malloc(st.st_size+1); if ((read(path_fp, path, st.st_size) == st.st_size) && path[0] == '/' && !stat(path, &st) && S_ISDIR(st.st_mode)){ freepath = path; path[st.st_size] = '\0'; } else { free(path); path = NULL; } } read() populates the path variable, but it is not null-terminated. So stat() fails because path ends with garbage. -- Guillaume LECERF GeeXboX developer - www.geexbox.org
On Tue, Jun 3, 2008 at 2:27 PM, Guillaume LECERF <foxcore@gmail.com> wrote:
freepath = path; path[st.st_size] = '\0';
Further simplifications. -- Guillaume LECERF GeeXboX developer - www.geexbox.org
Guillaume LECERF wrote:
On Tue, Jun 3, 2008 at 2:27 PM, Guillaume LECERF <foxcore@gmail.com> wrote:
freepath = path; path[st.st_size] = '\0';
Further simplifications.
No need to memset the whole path to 0. Just setting path[st.st_size] = 0 should be enough, as long as it's done before the call to stat(). Aurel
On Tue, Jun 3, 2008 at 7:33 PM, Aurelien Jacobs <aurel@gnuage.org> wrote:
Guillaume LECERF wrote:
On Tue, Jun 3, 2008 at 2:27 PM, Guillaume LECERF <foxcore@gmail.com> wrote:
freepath = path; path[st.st_size] = '\0';
Further simplifications.
No need to memset the whole path to 0. Just setting path[st.st_size] = 0 should be enough, as long as it's done before the call to stat().
Setting path[st.st_size] = 0 before the call to stat() implies splitting the test in two, which will result in a code duplication (2x free(path); path = null;) This patch is IMHO the less intrusive one, and btw reduce the code. What the problem with setting the whole path to 0 ? -- Guillaume LECERF GeeXboX developer - www.geexbox.org
Guillaume LECERF wrote:
On Tue, Jun 3, 2008 at 7:33 PM, Aurelien Jacobs <aurel@gnuage.org> wrote:
Guillaume LECERF wrote:
On Tue, Jun 3, 2008 at 2:27 PM, Guillaume LECERF <foxcore@gmail.com> wrote:
freepath = path; path[st.st_size] = '\0';
Further simplifications.
No need to memset the whole path to 0. Just setting path[st.st_size] = 0 should be enough, as long as it's done before the call to stat().
Setting path[st.st_size] = 0 before the call to stat() implies splitting the test in two
No. It implies setting path[st.st_size] = 0 before the whole test (before the if()).
What the problem with setting the whole path to 0 ?
It uselessly set st.st_size-1 bytes to 0. Aurel
On Tue, Jun 3, 2008 at 8:42 PM, Aurelien Jacobs <aurel@gnuage.org> wrote:
Setting path[st.st_size] = 0 before the call to stat() implies splitting the test in two
No. It implies setting path[st.st_size] = 0 before the whole test (before the if()).
OK, I got it now. The st struct is used twice, once for MENU_KEEP_PATH, and once for the path it contains. So I thought that the right st.size was the 2nd one. Then I realized that the size of MENU_KEEP_PATH and the length of the path is contains are actually equals.
What the problem with setting the whole path to 0 ?
It uselessly set st.st_size-1 bytes to 0.
I'm ok with you for a speed-critical section of the code, but in this case, it _should_ have been good enough. BTW patches attached (fix + cosmetic). -- Guillaume LECERF GeeXboX developer - www.geexbox.org
Guillaume LECERF a écrit :
On Tue, Jun 3, 2008 at 8:42 PM, Aurelien Jacobs <aurel@gnuage.org> wrote:
Setting path[st.st_size] = 0 before the call to stat() implies splitting the test in two No. It implies setting path[st.st_size] = 0 before the whole test (before the if()).
OK, I got it now. The st struct is used twice, once for MENU_KEEP_PATH, and once for the path it contains. So I thought that the right st.size was the 2nd one. Then I realized that the size of MENU_KEEP_PATH and the length of the path is contains are actually equals.
What the problem with setting the whole path to 0 ? It uselessly set st.st_size-1 bytes to 0.
I'm ok with you for a speed-critical section of the code, but in this case, it _should_ have been good enough.
BTW patches attached (fix + cosmetic).
Both applied. Ben
participants (3)
-
Aurelien Jacobs -
Benjamin Zores -
Guillaume LECERF