Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1375722
| From | Felipe Balbi <balbi@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | RE: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void |
| Date | 2016-04-11 13:30 +0200 |
| Message-ID | <rmIcy-51x-1@gated-at.bofh.it> (permalink) |
| References | <rlBd8-1LF-5@gated-at.bofh.it> <rlBd9-1LF-31@gated-at.bofh.it> <rmFeI-2HC-61@gated-at.bofh.it> <rmI2T-4Vk-35@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
Hi,
"Du, Changbin" <changbin.du@intel.com> writes:
>> > root = debugfs_create_dir(dev_name(dwc->dev), NULL);
>> > - if (!root) {
>> > - ret = -ENOMEM;
>> > - goto err0;
>> > - }
>> > + if (IS_ERR_OR_NULL(root))
>> > + return;
>>
>> We can definitely keep on going, but I'd still like to know that we
>> enabled CONFIG_DEBUG_FS but failed to create a file or a
>> directory. Seems like this should read as follows:
>>
>> if (IS_ERR_OR_NULL(root)) {
>> if (!root)
>> dev_err(dwc->dev, "Can't create debugfs root\n");
>> return;
>> }
>>
>> ditto to all bellow.
>>
> Balbi, so you mean we should not let the failure go silent, right?
yeah, but only iff CONFIG_DEBUG_FS is enabled. From what I can tell, in
case CONFIG_DEBUG_FS is enabled and it fails, it'll return a NULL
pointer instead of ERR_PTR() ;-)
>> > dwc->root = root;
>> >
>> > dwc->regset = kzalloc(sizeof(*dwc->regset), GFP_KERNEL);
>> > if (!dwc->regset) {
>> > - ret = -ENOMEM;
>> > - goto err1;
>> > + debugfs_remove_recursive(root);
>>
>> you're now duplicating debugfs_remove_recursive(root) in all braches and
>> that's error prone. It's probably better to keep our gotos, but change
>> them so they read as follows:
>>
>> if (!dwc->regset)
>> goto err1;
>>
>> [...]
>>
>> return; /* this is our successful exit point */
>>
>> err1:
>> debugfs_remove_recursive(root);
>> kfree(dwc->regset);
>>
>>
>> --
>> Balbi
>
> No, no need anymore. Because no branch share this code now.
> Then remove the goto would make code a little clear.
fair enough.
--
balbi
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-06 10:40 +0200
Re: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space Greg KH <gregkh@linuxfoundation.org> - 2016-04-06 11:30 +0200
RE: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-06 13:40 +0200
RE: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-06 14:30 +0200
[PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs changbin.du@intel.com - 2016-04-06 18:00 +0200
[PATCH v2 3/3] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-06 18:00 +0200
Re: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs Felipe Balbi <balbi@kernel.org> - 2016-04-07 07:10 +0200
RE: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs "Du, Changbin" <changbin.du@intel.com> - 2016-04-07 07:30 +0200
RE: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs Felipe Balbi <balbi@kernel.org> - 2016-04-07 07:30 +0200
[PATCH v3 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-08 11:50 +0200
[PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du@intel.com - 2016-04-08 11:50 +0200
Re: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void Felipe Balbi <balbi@kernel.org> - 2016-04-11 10:20 +0200
RE: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void "Du, Changbin" <changbin.du@intel.com> - 2016-04-11 13:20 +0200
RE: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void Felipe Balbi <balbi@kernel.org> - 2016-04-11 13:30 +0200
[PATCH v4 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-12 13:30 +0200
[PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-12 13:30 +0200
Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-04-12 15:00 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 05:30 +0200
Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 10:10 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 13:20 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 13:30 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 13:40 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 13:50 +0200
RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 14:00 +0200
[PATCH v4 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du@intel.com - 2016-04-12 13:30 +0200
[PATCH v3 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-08 11:50 +0200
csiph-web