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 returning argument types from methods. #3

Merged
merged 4 commits into from Sep 7, 2015

Conversation

ngrewe
Copy link
Member

@ngrewe ngrewe commented Sep 6, 2015

A tiny fix: method_copyArgumentType() didn't account for the fact that the return type is the first element in the method signature.

@@ -433,7 +433,7 @@ unsigned method_get_number_of_arguments(struct objc_method *method)
char* method_copyArgumentType(Method method, unsigned int index)
{
if (NULL == method) { return NULL; }
const char *types = findParameterStart(method->types, index);
const char *types = findParameterStart(method->types, index + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Does method_getArgumentType need the same fix? And please can we have a test case?

@ngrewe
Copy link
Member Author

ngrewe commented Sep 6, 2015

Does method_getArgumentType need the same fix?

It did indeed.

And please can we have a test case?

Included now.

@@ -393,7 +393,7 @@ void method_getArgumentType(Method method,
size_t dst_len)
{
if (NULL == method) { return; }
const char *types = findParameterStart(method->types, index);
const char *types = findParameterStart(method->types, index + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Rather than having the +1 in both places, it would probably be cleaner to fix findParaneterStart to start in the right place.

@ngrewe
Copy link
Member Author

ngrewe commented Sep 7, 2015

Good idea, thanks. Fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants