Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Jan 29, 2020

commit 39bd9ff670e552f33fc11e7513b63b86a75ee542
Author: YAMAMOTO Takashi <yamamoto@midokura.com>
Date:   Wed Jan 29 00:17:05 2020 +0900

	sim: Prefix symbols with _ for non-ELF

	Namely for Mach-O.  Leave __CYGWIN__ case as it is.

	commit 39bd9ff
	Author: YAMAMOTO Takashi <yamamoto@midokura.com>
	Date:   Wed Jan 29 00:17:05 2020 +0900

		sim: Prefix symbols with _ for non-ELF

		Namely for Mach-O.  Leave __CYGWIN__ case as it is.
@yamt yamt mentioned this pull request Jan 29, 2020
# define SYMBOL(s) _##s
#endif
#endif

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Jan 29, 2020

Choose a reason for hiding this comment

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

How about:

#ifdef __ELF__
# define SYMBOL(s) s
#else
# define SYMBOL(s) _##s
#endif

Like you change:

#ifdef __ELF__
	.type	SYMBOL(up_setjmp), @function
#endif

Also for up_setjmp64.S

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean to merge CYGWIN block?
CYGWIN seems to use _ prefix for 32-bit but not for 64-bit.
so I feel it's better to keep it separate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the separation is better.

@patacongo
Copy link
Contributor

@xiaoxiang781216 I think you should merge this when you are comfortable with the changes. I think only you and Yamamoto Takashi are sufficiently familiar with the build issues to make the correct technical decisions.

@yamt yamt mentioned this pull request Jan 29, 2020
@xiaoxiang781216 xiaoxiang781216 merged commit 2983fcd into apache:master Jan 29, 2020
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