Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
On Friday 19 July 2013, Dan Carpenter wrote:
debugfs_create_dir() returns ERR_PTR(-ENODEV) if debugfs is disabled.
Also my static checker doesn't like it when we print the error code, but
it's always just NULL.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
This looks wrong. debugfs_create_dir intentionally returns non-NULL so
failing to create the directory does not trigger an error condition if
debugfs is disabled.
Arnd
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 1b456fe..ad2cd6d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2215,7 +2215,7 @@ static int __init init(void)
}
pdrvdata.debugfs_dir = debugfs_create_dir(virtio-ports, NULL);
- if (!pdrvdata.debugfs_dir) {
+ if (IS_ERR_OR_NULL(pdrvdata.debugfs_dir)) {
pr_warning(Error %ld creating debugfs dir for
virtio-ports\n,
PTR_ERR(pdrvdata.debugfs_dir));
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
On Saturday 20 July 2013, Dan Carpenter wrote: On Fri, Jul 19, 2013 at 12:28:41PM +0200, Arnd Bergmann wrote: On Friday 19 July 2013, Dan Carpenter wrote: debugfs_create_dir() returns ERR_PTR(-ENODEV) if debugfs is disabled. Also my static checker doesn't like it when we print the error code, but it's always just NULL. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com This looks wrong. debugfs_create_dir intentionally returns non-NULL so failing to create the directory does not trigger an error condition if debugfs is disabled. Yeah. You're right. But the original code is still wrong and will oops if debugfs is disabled. We should set the pointer to NULL if we get a ERR_PTR(). I will send a v2 patch. I don't see where that oops would happen. In the code I'm looking at, all uses of -debugfs_dir only ever get passed into other debugfs functions that are stubbed out to empty inline functions. It's not the most obvious interface design, but this all seems intentional and correct to me. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
On Sun, Jul 21, 2013 at 11:36:25AM +0200, Arnd Bergmann wrote: On Saturday 20 July 2013, Dan Carpenter wrote: On Fri, Jul 19, 2013 at 12:28:41PM +0200, Arnd Bergmann wrote: On Friday 19 July 2013, Dan Carpenter wrote: debugfs_create_dir() returns ERR_PTR(-ENODEV) if debugfs is disabled. Also my static checker doesn't like it when we print the error code, but it's always just NULL. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com This looks wrong. debugfs_create_dir intentionally returns non-NULL so failing to create the directory does not trigger an error condition if debugfs is disabled. Yeah. You're right. But the original code is still wrong and will oops if debugfs is disabled. We should set the pointer to NULL if we get a ERR_PTR(). I will send a v2 patch. I don't see where that oops would happen. In the code I'm looking at, all uses of -debugfs_dir only ever get passed into other debugfs functions that are stubbed out to empty inline functions. It's not the most obvious interface design, but this all seems intentional and correct to me. It was the best interface design I could create, making it very easy for drivers to use and not really worry at all if debugfs was failing or not, or if it was even present in the system or not. That was the design goal I had for it when I wrote it. thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
I don't see where that oops would happen. In the code I'm looking at, all uses of -debugfs_dir only ever get passed into other debugfs functions that are stubbed out to empty inline functions. It's not the most obvious interface design, but this all seems intentional and correct to me. Ah. Ok. You're right. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
On Fri, Jul 19, 2013 at 12:28:41PM +0200, Arnd Bergmann wrote: On Friday 19 July 2013, Dan Carpenter wrote: debugfs_create_dir() returns ERR_PTR(-ENODEV) if debugfs is disabled. Also my static checker doesn't like it when we print the error code, but it's always just NULL. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com This looks wrong. debugfs_create_dir intentionally returns non-NULL so failing to create the directory does not trigger an error condition if debugfs is disabled. Yeah. You're right. But the original code is still wrong and will oops if debugfs is disabled. We should set the pointer to NULL if we get a ERR_PTR(). I will send a v2 patch. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [patch] virtio: console: fix error handling for debugfs_create_dir()
On (Fri) 19 Jul 2013 [08:50:49], Dan Carpenter wrote: debugfs_create_dir() returns ERR_PTR(-ENODEV) if debugfs is disabled. Also my static checker doesn't like it when we print the error code, but it's always just NULL. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Reviewed-by: Amit Shah amit.s...@redhat.com Today seems to be the 'fix virtio-console day'. Thanks, Amit ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
