Skip to content

const is fine on ares__channel_[un]lock - #758

Merged
bradh352 merged 2 commits into
c-ares:mainfrom
dimbleby:const-lock-is-fine
May 22, 2024
Merged

bradh352 merged 2 commits into
c-ares:mainfrom
dimbleby:const-lock-is-fine

Conversation

@dimbleby

Copy link
Copy Markdown
Contributor

at #601 (comment) you chose not to scatter const on the public interface because of the plan - now realised - to add threading to c-ares, and in the expectation that even read operations would need to lock the mutex.

But the threading implementation has a pointer to a mutex inside the ares channel and as I understand it, that means that it is just fine to mark ares__channel_lock (and ares__channel_unlock) as taking a const channel. It is the pointed-to mutex that is not constant, but C does not propagate const-ness through pointers.

Anyway here is a pull request limited to just that change, in the expectation that the pipelines will say so if I have misunderstood this.

This is not necessarily worth merging for itself, rather it is intended as a demonstration that - if you are still interested in it - #601 can be back on the table.

@bradh352

Copy link
Copy Markdown
Member

you're right, the thread implementation ended up mallocing a memory area for the OS-specific implementations rather than trying to create a system-specific typedef, such as to pthread_mutex_t (which would have made the original decision correct).

@bradh352

Copy link
Copy Markdown
Member

did you want to go ahead and sprinkle const on the relevant public functions with this PR too?

@dimbleby

Copy link
Copy Markdown
Contributor Author

sure, I can do that - might take me a day or two to get to it

@dfandrich

dfandrich commented May 20, 2024 via email

Copy link
Copy Markdown
Collaborator

@bradh352

Copy link
Copy Markdown
Member

@dfandrich are you sure? I mean on C++ it might change name mangling, but that's not a concern with C.

@dfandrich

dfandrich commented May 20, 2024 via email

Copy link
Copy Markdown
Collaborator

@dimbleby

Copy link
Copy Markdown
Contributor Author

updates pushed, pipeline green

looks as though I am using a more recent version of clang-format than was previously used, but since the pipeline doesn't mind I have just let it do its thing.

@bradh352
bradh352 merged commit 2f200b9 into c-ares:main May 22, 2024
@dimbleby
dimbleby deleted the const-lock-is-fine branch May 22, 2024 15:08
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.

3 participants