Bug #3860
ztest_vdev_aux_add_remove should use MAXPATHLEN instead of sizeof in snprintf()
0%
Description
GCC 4.8.0's new diagnostics detected a programming error in ztest.c when building ZFSOnLinux with on Gentoo Linux. I have attached a patch to fix it. Here are links to the downstream issue and the corresponding pull request:
https://github.com/zfsonlinux/zfs/issues/1480
https://github.com/zfsonlinux/zfs/pull/1561
On an unrelated note, I will probably refrain from filing issues for larger patches until I am comfortable enough with Illumos development to test them before filing issues.
Files
Updated by Richard Yao over 7 years ago
The title has a typo. It should be snprintf(), not sprintf().
Updated by Marcel Telka over 7 years ago
- Subject changed from ztest_vdev_aux_add_remove should use MAXPATHLEN instead of sizeof in sprintf() to ztest_vdev_aux_add_remove should use MAXPATHLEN instead of sizeof in snprintf()
Updated by
Updated by Igor Pashev over 7 years ago
Yuri Pankov wrote:
There's nothing wrong with using sizeof() that way here.
Unless path is a pointer:
https://github.com/ryao/zfs/blob/c236c99e097f596e85fd1b5de398ce908104a119/cmd/ztest/ztest.c#L2554
Updated by Richard Yao over 7 years ago
Yuri Pankov wrote:
There's nothing wrong with using sizeof() that way here.
Using sizeof on an dynamic allocation is wrong. Here is a small C program that you can compile:
#include <stdlib.h>
#include <stdio.h>int main (){
char *buffer = malloc(10240);
printf("sizeof buffer = %d\\n", sizeof buffer);
return 0;
}
The output on my system is "sizeof buffer = 8", which is the size of the pointer, not the size of the allocation. We should not be passing the size of the pointer to snprintf().
Updated by Richard Yao over 7 years ago
Igor, thanks for spotting that. Yuri was right. I neglected to notice that ZFSOnLinux had changed this to be a dynamic allocation from Illumos' static allocation when I checked if this patch applied to Illumos. The use of a stack allocation makes the original code fine. It was late at night when I filed this (and early in the morning now), so I missed that. Would someone close this issue? It is invalid.
Updated by