Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tool: Generate easysrc with last cache linked-list #452

Closed
wants to merge 1 commit into from

Conversation

@gnawhleinad
Copy link
Contributor

commented Sep 23, 2015

Using a last cache linked-list improves the performance of easysrc generation.

Bug: #444
Ref: #429

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 23, 2015

Nice!

... but since we have no user of this new list in the library I think it is premature to add it to the lib/ directory and instead we can make it locally in the src/ dir for now. Once/if we find a use for it in the library we can put it there.

@gnawhleinad gnawhleinad force-pushed the gnawhleinad:fix_444 branch Sep 24, 2015

@jay

This comment has been minimized.

Copy link
Member

commented Sep 24, 2015

In slist_wc_append_nodup check list for null like if(!list){free(new_item);return NULL;}. Also I don't understand why you have that check for invalid last node cache when can that happen? If never I'd get rid of all that and make it list->last->next = new_item; list->last = list->last->next;. Also why in curl_slist_wc_free_all are you calling curl_slist_free_all twice, I don't get what's happening there.

This reminds me a long time ago I wrote some C89 generic list macros that have head, tail, count. They're the most barebones thing you'll find, they have no type information which means danger, side effects... but they're purposely C89 compatible and the idea is to use the generic macros in type specific functions where they're (mostly) hidden from danger. I put it on github earlier this year. I don't see how it could make sense for this but https://github.com/jay/generic_list

@gnawhleinad gnawhleinad force-pushed the gnawhleinad:fix_444 branch Sep 24, 2015

@gnawhleinad

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2015

Appreciate all the feedback!

@jay

This comment has been minimized.

Copy link
Member

commented Sep 25, 2015

You can take advantage of slist's facilities to create the item:

  struct curl_slist *new_item = curl_slist_append(NULL, data);

  if(!new_item)
    return NULL;
tool: Generate easysrc with last cache linked-list
Using a last cache linked-list improves the performance of easysrc generation.

Bug: #444
Ref: #429

@gnawhleinad gnawhleinad force-pushed the gnawhleinad:fix_444 branch to 2717cc8 Sep 25, 2015

@bagder bagder self-assigned this Oct 17, 2015

@bagder bagder added the cmdline tool label Oct 17, 2015

@bagder bagder closed this in 19cb0c4 Oct 17, 2015

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.