buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50325, DNS local host list exported...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #50325: DNS local host list exported functions lack versatility

Submitted by:  Konstantin <tomcat_84>
Submitted on:  Thu 16 Feb 2017 09:54:25 AM UTC  
 
Category:  DNS Severity:  3 - Normal
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
lwIP version:  2.0.0

( Jump to the original submission)

Fri 17 Feb 2017 09:09:23 PM UTC, comment #11: 

I had a talk with Dirk about the iterator not providing an 'idx' in the callback. However, since the table can be 'const' (a const array in ROM) or 'dynamic' (a list with next pointers), we cannot just export the table (since it would be different for the const/dynamic cases.

If anyone has suggestions for improvement of the change, feel free to post here.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 17 Feb 2017 02:31:24 PM UTC, comment #10: 

Thanks.

Konstantin <tomcat_84>
Fri 17 Feb 2017 09:10:20 AM UTC, comment #9: 
Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 17 Feb 2017 07:29:22 AM UTC, comment #8: 

Agree about TTL. It is not for locals list.

Yes, iterator would also solve the problem. Publishing already implemented method would be a bit less complicated than organizing the callback functionality both for DNS and for user, but the latter allows for much more than simple search. Arguments like name-ip pair pointers seem better for such a callback than table entry struct pointer, for not to allow to mess with the "next" element in the struct.

Thank you.

Konstantin <tomcat_84>
Thu 16 Feb 2017 07:38:22 PM UTC, comment #7: 

To make it clear, I think TTL is out of scope for the "local dns" feature. It's meant to get locally configured, static entries into the table (much like the "hosts" file on some OSes). Anything beyond that should use DNS_LOOKUP_LOCAL_EXTERN(..).

Keeping the TTL aside, I agree that it would be good to have the possibility to get the current entries from the local list, e.g. to show the user a list on add/remove without having to keep the data twice.

An iterator might be better for that though. Something like this:

u8_t dns_local_iterate(dns_found_callback iterator);

That would call the 'iterator' function for every entry in the local table and return the number of entries.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 16 Feb 2017 03:48:41 PM UTC, comment #6: 

Of course, the user program could implement its own list (with add, remove, lookup and even TTLs), just using dns_local_addhost, when the user list grows, and dns_local_removehost, when entry expire due to TTL.
But in my implementation this one seems more like waste of time writing implementation (which is mostly already inside dns.c local list functions), and waste of code and data memory as well as some CPU processing time (because list and list operations are doubled), while published lookup would reduce the job greatly.

Please, I would not want to be misunderstood, I may still use one solution or another, and the whole topic is just a suggestion for the future versions, which could make it easier for others. It is not critical / on fire.

Konstantin <tomcat_84>
Thu 16 Feb 2017 03:30:56 PM UTC, comment #5: 

Agree with undocoumented non-static as not a good choice.

I understand it was meant to be implemented so. I just suggest that publishing already implemented lookup may have its uses, since it does read-only job and cannot affect performance or stability more than already published addhost and removehost.

Suppose we are running about 30 minutes, and user program have acquired hostname-ip pair in a way other than through "dns.c". It wants to inform LwIP DNS, which seems to be easy through dns_local_addhost, since it allows to specify both pair parameters (even if the IP is not really "local"). Using published lookup it could determine, whether it is already on the list or not (to avoid duplicates). If it wants to get the cached IP, it could use the same lookup.

Maybe I misunderstand the local list purpose, but it is the only way I have found to tell LwIP DNS at runtime about hostname-ip pair. The pair user program is sure of, but DNS had no idea, because user program has got it in some other way.

Maybe it would be better to solve through other published functions, which would allow user program to add/remove entries to DNS cache table instead of DNS local list, this way Add function with TTL specified could be used. But this latter solution would be much more complicated...

Konstantin <tomcat_84>
Thu 16 Feb 2017 02:37:12 PM UTC, comment #4: 

Undocumented non-static is a no-go. Good compilers warn you about that.

The local thing is meant as it is implemented. I don't really get what you want to do when having it "public". Do you want to iterate the existing entries?

Maybe DNS_LOOKUP_LOCAL_EXTERN() is more what you want?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 16 Feb 2017 01:02:51 PM UTC, comment #3: 

Sorry for cofusing names, it was all about exported

"dns_local_removehost"
"dns_local_addhost"

and static

"dns_lookup_local"

which, if you decide it possible, may be useful to user (e.g. renamed to non-static "dns_local_lookup" or "dns_local_search"). Whether it is brought to h-file or remained as undocumented non-static, both choices may do.

Konstantin <tomcat_84>
Thu 16 Feb 2017 11:38:39 AM UTC, comment #2: 

I can't say I understand what you mean...

I can't find "dns_localhost_lookup" or "dns_localhost" in our code.

You're either confusing names or you are using a 3rd party version of lwIP.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 16 Feb 2017 10:11:52 AM UTC, comment #1: 

Sorry, for some reason the form posted without any contents except header, with no edit possibility found I post it as a following comment.

DNS local host list seems to narrow to Add and Remove exported functions, which neither allow for TTL implementation (which is not so important in my case) nor give navigation and read options (which is much regrettable). Moreover, any referred functions and variables in "dns.c" are for some reason declared as "static".

This not only hinders keeping track of what was dynamically added there, to avoid duplicates. For a system with other name-IP pair source, keeping track of them is a problem, but it is further complicated if dynamic mechanism is already somewhere inside compiled code but hidden by static declarations, which demands to either write code and keep space for another list or edit "dns.c" in order to at least remove "static" modifier.

Suggest removing "static" modifier for dns_localhost_lookup, or expand the dns_localhost interface in some other way it becomes more of use for something other than just set constants.

Konstantin <tomcat_84>
Thu 16 Feb 2017 09:54:25 AM UTC, original submission:  
Konstantin <tomcat_84>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by tomcat_84 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-17 goldsimon StatusNone => Fixed
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed

    Back to the top


    Powered by Savane 3.5