Skip to content

Conversation

@SolidWallOfCode
Copy link
Member

@SolidWallOfCode SolidWallOfCode commented May 1, 2020

For issue #6716.

Two commits.

  • Trivial changes to fix errors.
  • Redo traffic_via to use string_view, which fixes an error which would otherwise be more painful to deal with.

@SolidWallOfCode SolidWallOfCode requested review from bryancall and zwoop May 1, 2020 19:31
@SolidWallOfCode SolidWallOfCode self-assigned this May 1, 2020
@SolidWallOfCode SolidWallOfCode added this to the 10.0.0 milestone May 1, 2020
int cleanup(int event, Event *e);

PinnedDocTable() : Continuation(new_ProxyMutex()) { memset(bucket, 0, sizeof(Queue<PinnedDocEntry>) * PINNED_DOC_TABLE_SIZE); }
PinnedDocTable() : Continuation(new_ProxyMutex()) { ink_zero(bucket); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this initialization? Queue's constructor should be called as long as this constructor is called.

num_consumers = 0;
memset(consumers, 0, sizeof(consumers));
memset(producers, 0, sizeof(producers));
ink_zero(consumers);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a right way to solve the problem. I'd use placement new for each item.

@SolidWallOfCode
Copy link
Member Author

@maskit I understand your points, but I think those should be separate PRs. For this one, I tried to minimize changes. Unfortunately I had to do some non-trivial reworking in traffic_via, but I'd rather leave these as is and fix them independently.
@bryancall I'll get that fixed.

@bryancall
Copy link
Contributor

[approve ci autest]

@bryancall bryancall merged commit a1fb39e into apache:master May 4, 2020
@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 May 5, 2020
@zwoop
Copy link
Contributor

zwoop commented May 5, 2020

Cherry-picked to v9.0.x branch.

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.

4 participants