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

bugfix #9

Open
basetutu opened this issue Oct 8, 2018 · 2 comments
Open

bugfix #9

basetutu opened this issue Oct 8, 2018 · 2 comments

Comments

@basetutu
Copy link

basetutu commented Oct 8, 2018

sglib/sglib.h

Line 554 in 1d17ed7

if ((elem)->previous != NULL) (elem)->previous->next = (elem);\

Hi!
Would like to suggest a change. (bugfix)
After the indicated line above, there should be the line:
(place) = (elem)\

This allows consecutive use of sglib_type_add_before(dllist_first), in the corner case where the dllist_first is NULL to begin with.

Current solution results in keeping the dllist_first pointer value the same as first call and place all new entries behind this first one which is continuously is moving further away from the 'front'.

Hence, new entries are placed at length-2 position!

This change fixed it. I can also push the change for review, if you like.
I am only working with the doubly ended linked list.

Best regards

@rhizoome
Copy link
Contributor

rhizoome commented Oct 8, 2018

Hi @basetutu

First, this repository is not maintained, either by me or @stefanct. Second, I sent my patches to Marian Vittek, and he isn't around anymore. I ended up doing my own implementation (only rbtree, queue and stack; I don't see the point of linked lists).

If I were the maintainer of sglib, I would ask you to extend the tests for this edge cases, to demonstrate how the patch improves things. So I can step through and convince myself, that there are no negative side-effects.

I like sglib very much, that's the reason I implemented parts of it from scratch, in order to learn how to implement generic C libraries. However, I won't maintain sglib. That is the sad truth.

Peace, out.

@rhizoome
Copy link
Contributor

rhizoome commented Oct 8, 2018

@basetutu I think I did almost the same thing here:

1d17ed7

Maybe with your change my change should be reverted. Because your fix is general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants