Skip to content

Conversation

stooke
Copy link
Contributor

@stooke stooke commented Oct 30, 2018

Hi, and thanks for all the work on HarfBuzz.
I've been working to reduce warnings from the latest GCC, which is fairly strict.
This patch fixes some function parameter / template and other issues.
It'd be great if you consider accepting it.

This patch fixes these warnings:

....r/harfbuzz/hb-debug.hh:130:1: error: explicit specialization ‘void _hb_debug_msg_va(const char*, const void*, const char*, bool, unsigned int, int, const char*, __va_list_tag*) [with int max_level = 0]’ may be missing attributes [-Werror=missing-attributes]
_hb_debug_msg_va<0> (const char *what HB_UNUSED,

and:
.../harfbuzz/hb-set-private.hh:54:60: error: ‘void* memset(void*, int, size_t)’ writing to an object of type ‘hb_set_t::page_t::vector_t’ {aka ‘struct hb_vector_size_t<long long unsigned int, 64>’} with ‘private’ member ‘hb_vector_size_t<long long unsigned int, 64>::u’ [-Werror=class-memaccess]
inline void init1 (void) { memset (&v, 0xff, sizeof (v)); }

@behdad behdad merged commit 881e105 into harfbuzz:master Oct 30, 2018
@behdad
Copy link
Member

behdad commented Oct 30, 2018

Not sure about the reinterpret_cast. Why did you need it? memset takes void*, and that should be automatic? Even then, why cast to char* instead of void* then? We typically just use C-style casts.

@stooke
Copy link
Contributor Author

stooke commented Oct 31, 2018

Thanks for merging my PR. You're absolutely right, while my cast works, casting to void* using a C-style cast will work too.

The warning is not that the type of the parameter is wrong, but that it's modifying a private member.
I use reinterpret_cast to show the code is doing something potentially quite dangerous, but I should have used void* instead of char* (the end result is the same, but yours is more correct).

I'm more than happy to submit a simple PR to change this to C-style cast to void*, and I'm sorry I didn't match your style to begin with - just let me know.

@ebraminio
Copy link
Collaborator

Yes. Feel free to do so :)

@behdad
Copy link
Member

behdad commented Oct 31, 2018

The warning is not that the type of the parameter is wrong, but that it's modifying a private member.

Ah, interesting. Maybe I rewrite the code to call a method instead. That makes sense.

behdad added a commit that referenced this pull request Oct 31, 2018
@behdad
Copy link
Member

behdad commented Oct 31, 2018

Rewote the code. Please test.

@stooke
Copy link
Contributor Author

stooke commented Oct 31, 2018

Your new code seems to be fine, thanks.

ebraminio pushed a commit to ebraminio/harfbuzz that referenced this pull request Nov 24, 2018
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