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

A question in sdsIncrLen #39

Open
aronlt opened this issue Nov 14, 2014 · 4 comments
Open

A question in sdsIncrLen #39

aronlt opened this issue Nov 14, 2014 · 4 comments

Comments

@aronlt
Copy link

aronlt commented Nov 14, 2014

In the function sdsIncrLen, I think the statement "assert(sh->free >= 0) " is redundant, because "assert(sh->free >= incr)" makes sure this situation would not happen.

void sdsIncrLen(sds s, int incr) {
struct sdshdr sh = (void)(s - sizeof *sh);
assert(sh->free >= incr);
sh->len += incr;
sh->free -= incr;
assert(sh->free >= 0);
s[sh->len] = '\0';
}

@kyrias
Copy link

kyrias commented Jan 20, 2015

Yeah, the only thing that could make it be less than 0 would be in a multithreaded programs where two threads are trying to modify the same string at once, but that’d be very broken.

Anyway, @antirez seems to have no intention of maintaining SDS and I emailed him a couple of months ago about it but he never replied, so I’m working on a fork currently located at https://github.com/yabok/sds, though it will be moved to (most likely) https://github.com/yabok/yasl soon.

@lemzwerg
Copy link

See pull request #45 for a fix.

@kyrias
Copy link

kyrias commented Feb 15, 2015

@lemzwerg If you look at the repo history and the other PRs and issues open you can see how it's pretty useless to open a PR on this repo.

@lemzwerg
Copy link

Maybe, yes. However, the PRs I produce are just a byproduct and quite easy to generate, so why not. I'm used to normally commit code with associated ChangeLog entries; this essentially means that you have to concisely wrap up your modifications in an atomic way – in most cases this already corresponds to a PR...

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

3 participants