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

bitvector pull request #53

Closed
wants to merge 3 commits into from
Closed

bitvector pull request #53

wants to merge 3 commits into from

Conversation

trackpick
Copy link

Hi Eddie,

A small bug fix for you. Let me know if it looks OK - this is my first pull request!

 Cheers,
 Patrick

Patrick Verkaik and others added 3 commits August 9, 2012 21:26
In the case that the lhs bitvector is shorter than the rhs bitvector, operator|=
tries to 'or' in words past the end of the rhs bitvector (presumably
uninitialized memory).
Right now it does. Next commit fixes this.

git-svn-id: svn+ssh://svn.meraki.net/var/local/svnbase/meraki/trunk/router@83372 291e08f9-2c14-0410-acb0-c89b6de265c5
I got a coredump when calling the capacity write handler on an empty queue.  I'm
not 100% confident about this change, since I haven't been through all of the
code.

git-svn-id: svn+ssh://svn.meraki.net/var/local/svnbase/meraki/trunk/router@83373 291e08f9-2c14-0410-acb0-c89b6de265c5
@kohler
Copy link
Owner

kohler commented Aug 10, 2012

Hey Patrick,

I found another Bitvector bug while looking over your change, and fixed it.
I think you will want to integrate it, along with the associated tests.

Best
E

On Fri, Aug 10, 2012 at 1:18 AM, Patrick Verkaik
notifications@github.comwrote:

Hi Eddie,

A small bug fix for you. Let me know if it looks OK - this is my first
pull request!

Cheers,
Patrick


You can merge this Pull Request by running:

git pull https://github.com/trackpick/click master

Or view, comment on, or merge it at:

#53
Commit Summary

  • bitvector: fix operator|= bug

File Changes

  • M lib/bitvector.cc (4)

Patch Links

@kohler kohler closed this Aug 10, 2012
@trackpick
Copy link
Author

Hey Eddie,

Wow, I really screwed up this pull request. Not sure how I ended up including the frontdropqueue changes in there.

I see you've added a test already. I found the bug in a testie for a Meraki element that I wrote, so it wasn't appropriate for mainline. (The element has a couple of bitmap for VLANs and I was getting crazy VLANs in my set after doing a union.) Next time I'll write a separate testie.

I'll pull your other fixes for Bitvector. Thanks!

Patrick

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

Successfully merging this pull request may close these issues.

2 participants