[GitHub] [incubator-nuttx] anchao commented on pull request #3102: mm/iob: add iob_free_queue() interface

[GitBox <gi...@apache.org>]

anchao commented on pull request #3102:
URL: https://github.com/apache/incubator-nuttx/pull/3102#issuecomment-822272855


   > @xiaoxiang781216 @anchao
   > 
   > I've now spent some more time looking at this and I still think we need to improve the naming. Firstly, free is the better term to use when we are freeing up a resource to be used again. Destroy makes it sound like the resource doesn't exist after, which is not true.
   > 
   > What you have now named iob_free_queue() - the implementation of which did not exist before, finds the given IOB in the queue and removes it, freeing both the IOB chain itself, and the qentry. Therefore, I propose you name this function one of the following:
   > 
   > * iob_free_qentry_from_queue() <- Long but very descriptive
   > * iob_freeone_queue()
   > * iob_queue_freeone()
   > 
   > I would therefore revert the renaming of the original implementation of iob_free_queue(), as I actually think that name was descriptive of what the function did.
   > 
   > Let me know your thoughts - I'm open to other naming but I don't think this change as it stands is very readable.
   
   
   
   > @xiaoxiang781216 @anchao
   > 
   > I've now spent some more time looking at this and I still think we need to improve the naming. Firstly, free is the better term to use when we are freeing up a resource to be used again. Destroy makes it sound like the resource doesn't exist after, which is not true.
   > 
   > What you have now named iob_free_queue() - the implementation of which did not exist before, finds the given IOB in the queue and removes it, freeing both the IOB chain itself, and the qentry. Therefore, I propose you name this function one of the following:
   > 
   > * iob_free_qentry_from_queue() <- Long but very descriptive
   > * iob_freeone_queue()
   > * iob_queue_freeone()
   > 
   > I would therefore revert the renaming of the original implementation of iob_free_queue(), as I actually think that name was descriptive of what the function did.
   > 
   > Let me know your thoughts - I'm open to other naming but I don't think this change as it stands is very readable.
   
   Hi @antmerlino 
   
   Sorry for reply late, thanks for your suggestion, 
   
   the api naming your mentioned seems very detailed and easy to understand, but which out of tune with other iob api naming...
   how about change the iob_free_queue() to iob_release_queue() ? 
   
   ```
   rename: iob_free_queue() -> iob_release_queue()  // enqueue the entire iob/chain entries to freelist
   new   : iob_free_queue()                         // enqueue the specified iob/chain entries to freelist
   ```


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org