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

Add a non-const overload of bytearray::data #771

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

tjstum
Copy link
Contributor

@tjstum tjstum commented Oct 29, 2024

Unlike bytes, a bytearray is mutable. You are allowed to write to it, and the underlying API function returns a char*

@tjstum
Copy link
Contributor Author

tjstum commented Oct 29, 2024

This seemed simple enough to skip a test and changelog entry, but I'm happy to add those if that's desired. Thanks!

Copy link

@Hnasar Hnasar left a comment

Choose a reason for hiding this comment

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

LGTM

@wjakob
Copy link
Owner

wjakob commented Nov 3, 2024

In this case, I would expect to have a const and a non-const version of the cast operator, with only the latter having a non-const return value.

@tjstum
Copy link
Contributor Author

tjstum commented Nov 3, 2024

Thanks for the feedback! Just to be clear, since these are named methods, not operators, are you saying that you'd prefer if this adds c_str_mutable() and data_mutable() functions to the bytearray class?

@oremanj
Copy link
Contributor

oremanj commented Nov 3, 2024

I imagine the desire was for overloading on constness:

char* data() { return …; }
const char* data() const { return …; }

I would personally keep the c_str name const-only, but it would also be possible to do the same with it.

Unlike `bytes`, a `bytearray` is mutable. You are allowed to write to
it, and the underlying API function returns a `char*`

This adds an additional overload for the non-const version when called
on a non-const this
@tjstum tjstum changed the title Return bytearray buffers as non-const Add a non-const overload of bytearray::data Nov 4, 2024
@wjakob
Copy link
Owner

wjakob commented Nov 5, 2024

Thanks!

@wjakob wjakob merged commit 418febe into wjakob:master Nov 5, 2024
31 checks passed
@tjstum tjstum deleted the bytearray branch November 5, 2024 17:07
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