Skip to content

Macro wrapper for memcpy#13770

Merged
jmarantz merged 122 commits intoenvoyproxy:mainfrom
rialg:issue-9328
Mar 16, 2021
Merged

Macro wrapper for memcpy#13770
jmarantz merged 122 commits intoenvoyproxy:mainfrom
rialg:issue-9328

Conversation

@rialg
Copy link
Contributor

@rialg rialg commented Oct 27, 2020

Description:

Initial migration from memcpy to SAFE_MEMCPY Macro and MemBlockBuilder. Only a few files were changed, tests and corrections to the check_format script are still pending.

Fixes #9328

grial added 2 commits October 25, 2020 22:39
@rialg rialg requested a review from mattklein123 as a code owner October 27, 2020 00:26
grial added 8 commits October 27, 2020 09:44
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
…einterpret_cast

Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
grial added 7 commits October 29, 2020 09:55
Signed-off-by: grial <grial@google.com>
This reverts commit 6e3a257.

Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
grial added 5 commits October 30, 2020 19:49
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
Signed-off-by: grial <grial@google.com>
…r_impl

Signed-off-by: grial <grial@google.com>
Copy link
Contributor Author

@rialg rialg left a comment

Choose a reason for hiding this comment

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

Hi, I failed see how to change line 189 (memcpy(base_ + data_, src + size - copy_size, copy_size);) so as to implement it with either SAFE_MEMCPY or MemBlockBuilder. WDYT?

Signed-off-by: grial <grial@google.com>
@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13770 (comment) was created by @grial1.

see: more, trace.

jmarantz
jmarantz previously approved these changes Jan 11, 2021
Base automatically changed from master to main January 15, 2021 23:01
@jmarantz
Copy link
Contributor

main merge needed.

Signed-off-by: grial1 <grial@google.com>
jmarantz
jmarantz previously approved these changes Jan 22, 2021
@jmarantz
Copy link
Contributor

@lizan do you have other comments? It would be good to get this in as it's kind of a conflict-magnet :)

@rialg
Copy link
Contributor Author

rialg commented Jan 23, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13770 (comment) was created by @rialg.

see: more, trace.

Signed-off-by: grial1 <grial@google.com>
lizan
lizan previously approved these changes Feb 5, 2021
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Sorry I thought I approved this one, can you fix CI then I can merge.

@lizan
Copy link
Member

lizan commented Feb 5, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13770 (comment) was created by @lizan.

see: more, trace.

@jmarantz
Copy link
Contributor

Hi @rialg -- can you 'merge main' and get CI sorted? Then @lizan can merge.

Signed-off-by: rialg <grial@google.com>
@rialg
Copy link
Contributor Author

rialg commented Feb 27, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13770 (comment) was created by @rialg.

see: more, trace.

@jmarantz jmarantz dismissed htuch’s stale review March 16, 2021 23:11

all changes addressed

@jmarantz jmarantz merged commit 7852c7b into envoyproxy:main Mar 16, 2021
@rialg rialg deleted the issue-9328 branch March 21, 2021 09:57
@rialg
Copy link
Contributor Author

rialg commented Mar 21, 2021

ACK. Thanks @jmarantz!

@jmarantz
Copy link
Contributor

And thank you for pushing this all the way through! IIRC there were some follow-ups from this PR. Are you going to be able to pursue those?

@rialg
Copy link
Contributor Author

rialg commented Mar 21, 2021

Of course :). I will start to work on the pending changes on a new branch and submit the PR for reviewing next week, most likely.

rialg pushed a commit to rialg/envoy that referenced this pull request Mar 27, 2021
…eference to memory that has been already allocated

Signed-off-by: rialg <grial@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api deps Approval required for changes to Envoy's external dependencies v2-freeze

Projects

None yet

Development

Successfully merging this pull request may close these issues.

util: memcpy of whole structures should have a macro wrapper

4 participants