On Fri, Nov 29, 2024 at 10:00:30PM +0530, Manivannan Sadhasivam wrote:
On Fri, Nov 29, 2024 at 07:51:30PM +0900, Damien Le Moal wrote:
On 11/29/24 18:24, Manivannan Sadhasivam wrote:
IOCTLs are supposed to return 0 for success and negative error codes for failure. Currently, this driver is returning 0 for failure and 1 for success, that's not correct. Hence, fix it!
Reported-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Closes: https://lore.kernel.org/all/YvzNg5ROnxEApDgS@kroah.com Fixes: 2c156ac71c6b ("misc: Add host side PCI driver for PCI test function device") Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
Looks OK to me.
Reviewed-by: Damien Le Moal dlemoal@kernel.org
One nit below.
[...]
static void pci_endpoint_test_remove(struct pci_dev *pdev) diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c index 470258009ddc..545e04ad63a2 100644 --- a/tools/pci/pcitest.c +++ b/tools/pci/pcitest.c @@ -16,7 +16,6 @@ #include <linux/pcitest.h> -static char *result[] = { "NOT OKAY", "OKAY" }; static char *irq[] = { "LEGACY", "MSI", "MSI-X" }; struct pci_test { @@ -52,63 +51,65 @@ static int run_test(struct pci_test *test) ret = ioctl(fd, PCITEST_BAR, test->barnum); fprintf(stdout, "BAR%d:\t\t", test->barnum); if (ret < 0)
fprintf(stdout, "TEST FAILED\n");
elsefprintf(stdout, "NOT OKAY\n");
fprintf(stdout, "%s\n", result[ret]);
fprintf(stdout, "OKAY\n");
Maybe replace all this "if (ret < 0) ... else ..." and all the ones below with something a call to:
static void test_result(int ret) { fprintf(stdout, "%sOKAY\n", ret < 0 ? "NOT " : ""); }
or simply with the call:
fprintf(stdout, "%sOKAY\n", ret < 0 ? "NOT " : "");
to avoid all these repetition.
Sounds good to me. Will incorporate in next version, thanks!
Maybe not. This test is converted to Kselftest in successive patches, so no need to simplify it.
- Mani