You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/11 16:13:59 UTC
[GitHub] [incubator-nuttx] hartmannathan commented on a change in pull request #5718: drivers/can: optimize can driver reader side
hartmannathan commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-nuttx/pull/5718#discussion_r824860390
##########
File path: drivers/can/can.c
##########
@@ -407,46 +406,48 @@ static int can_open(FAR struct file *filep)
* for this device, then perform hardware initialization.
*/
- if (list_is_empty(&dev->cd_readers))
+ caninfo("ocount: %u\n", dev->cd_crefs);
+
+ if (dev->cd_crefs >= 255)
{
- caninfo("ocount: %d\n", 0);
+ /* Limit to no more than 255 opens */
+ ret = -EMFILE;
+ goto errout;
+ }
+ else
+ {
flags = enter_critical_section();
- ret = dev_setup(dev);
- if (ret >= 0)
+
+ if (dev->cd_crefs == 0)
{
- /* Mark the FIFOs empty */
+ ret = dev_setup(dev);
+ if (ret == OK)
+ {
+ /* Mark the FIFOs empty */
- dev->cd_xmit.tx_head = 0;
- dev->cd_xmit.tx_queue = 0;
- dev->cd_xmit.tx_tail = 0;
+ dev->cd_xmit.tx_head = 0;
+ dev->cd_xmit.tx_queue = 0;
+ dev->cd_xmit.tx_tail = 0;
- /* Finally, Enable the CAN RX interrupt */
+ /* Finally, Enable the CAN RX interrupt */
- dev_rxint(dev, true);
+ dev_rxint(dev, true);
+ }
}
- list_add_head(&dev->cd_readers,
- (FAR struct list_node *)init_can_reader(filep));
-
- leave_critical_section(flags);
- }
- else
- {
- tmp = list_length(&dev->cd_readers);
- caninfo("ocount: %d\n", tmp);
-
- if (tmp >= 255)
+ if (ret == OK)
Review comment:
This ret may be the one from either can_takesem() or dev_setup(). Not saying that's a problem, just saying.
##########
File path: drivers/can/can.c
##########
@@ -407,46 +406,48 @@ static int can_open(FAR struct file *filep)
* for this device, then perform hardware initialization.
*/
- if (list_is_empty(&dev->cd_readers))
+ caninfo("ocount: %u\n", dev->cd_crefs);
+
+ if (dev->cd_crefs >= 255)
{
- caninfo("ocount: %d\n", 0);
+ /* Limit to no more than 255 opens */
Review comment:
LGTM; we were missing this before and could rollover if too many opens.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org