Skip to content

zend defines attribute malloc for Win32 as returned pointer are not a… #9118

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

Closed
wants to merge 1 commit into from

Conversation

devnexen
Copy link
Member

…liased

@iluuu1994
Copy link
Member

The attribute looks fitting. Unfortunately, Microsoft seems to expect the attribute before the function signature though, whereas for CGG/Clang it's after. Not sure how we should handle that.

@cmb69
Copy link
Member

cmb69 commented Jul 23, 2022

A good idea, but I don't think it can work this way, since __declspec() needs to be at the beginning of the definition, while ZEND_ATTRIBUTE_MALLOC is currently at the end. :(

@devnexen devnexen force-pushed the define_malloc_attr_win32 branch from c058f51 to 7e3c671 Compare July 23, 2022 14:15
@iluuu1994
Copy link
Member

iluuu1994 commented Jul 23, 2022

@devnexen Personally I'd prefer if we added a new macro for __declspec(restrict) that was empty for non-Windows systems. This has two benefits:

  1. We don't need duplication of the function signatures
  2. Existing usages of ZEND_ATTRIBUTE_MALLOC won't break, as without the #ifndef ZEND_WIN32 they'd be in the wrong spot

@devnexen
Copy link
Member Author

Sure we could do that way, I wanted not to add more noise to an already noisy declaration set especially some of them don t apply to windows :) but I ll follow your point.

@devnexen devnexen force-pushed the define_malloc_attr_win32 branch from 7e3c671 to 2cc6b23 Compare July 23, 2022 16:57
@cmb69
Copy link
Member

cmb69 commented Jul 24, 2022

Do we really need a new macro here? Wouldn't ZEND_ATTRIBUTE_MALLOC work, if we put it at the beginning of the function declarations?

@devnexen devnexen force-pushed the define_malloc_attr_win32 branch from 2cc6b23 to 862b4f2 Compare July 24, 2022 18:16
@devnexen devnexen force-pushed the define_malloc_attr_win32 branch from 862b4f2 to c42a33a Compare July 24, 2022 18:34
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Thanks! I wasn't aware GCC/Clang accept the attribute in both places.

@devnexen devnexen closed this in 53ae24e Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants