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

fix buffer read/write #576

Merged
merged 10 commits into from
May 12, 2024
Merged

fix buffer read/write #576

merged 10 commits into from
May 12, 2024

Conversation

salix5
Copy link
Collaborator

@salix5 salix5 commented Apr 15, 2024

https://gist.github.com/shafik/848ae25ee209f698763cffee272a58f8
According to the strict aliasing rule, we should not cast a pointer should to another type and dereference.
It can be done by memcpy.

C++14
3.10, paragraph 10

If a program attempts to access the stored value of an object through a glvalue of other than one of the
following types the behavior is undefined:
—(10.1) the dynamic type of the object,
—(10.2) a cv-qualified version of the dynamic type of the object,
—(10.3) a type similar (as defined in 4.4) to the dynamic type of the object,
—(10.4) a type that is the signed or unsigned type corresponding to the dynamic type of the object,
—(10.5) a type that is the signed or unsigned type corresponding to a cv-qualified version of the dynamic type
of the object,
—(10.6) an aggregate or union type that includes one of the aforementioned types among its elements or non-
static data members (including, recursively, an element or non-static data member of a subaggregate
or contained union),
—(10.7) a type that is a (possibly cv-qualified) base class type of the dynamic type of the object,
—(10.8) a char or unsigned char type.

Add buffer.h

Reference:
https://github.com/edo9300/edopro/blame/master/gframe/bufferio.h

@salix5 salix5 mentioned this pull request Apr 20, 2024
@salix5 salix5 changed the title use memcpy in ocgapi.cpp fix buffer read/write Apr 25, 2024
@salix5 salix5 requested a review from mercury233 April 25, 2024 15:58
@salix5
Copy link
Collaborator Author

salix5 commented May 10, 2024

@mercury233
@purerosefallen
Can we merge this one?

@salix5 salix5 merged commit 607a170 into master May 12, 2024
1 check passed
@salix5 salix5 deleted the patch-aliasing branch May 12, 2024 13:18
@purerosefallen
Copy link
Collaborator

I doubt if reconnecting works

@salix5
Copy link
Collaborator Author

salix5 commented May 12, 2024

I only changed buffer writing to the way it should be.
I don't think it will change reconnecting.

@salix5 salix5 removed the request for review from mercury233 May 18, 2024 05:13
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