| Message ID | 20180607120321.2548-1-martin@martin.st |
|---|---|
| State | Committed |
| Commit | 52fd2afce8436c59c05765f3a6e95f9adb6f9f2f |
| Headers | show |
| Series |
|
| Related | show |
On 07/06/2018 14:03, Martin Storsjö wrote: > From: John Cox <jc@kynesim.co.uk> > > Commit 8c893aa3cd5 removed quotes that were required to detect > inline asm: > > check_insn armv5te qadd r0, r0, r0 > .../test.c:1:34: error: expected string literal in 'asm' > void foo(void){ __asm__ volatile(qadd r0, r0, r0); } > > The correct code is: > > void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } > > Commit message written by Frank Liberato <liberato@chromium.org> > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 5e79c0cec1..48e8536b07 100755 > --- a/configure > +++ b/configure > @@ -866,7 +866,7 @@ EOF > > check_insn(){ > log check_insn "$@" > - check_inline_asm ${1}_inline "$2" > + check_inline_asm ${1}_inline "\"$2\"" > check_as ${1}_external "$2" > } > > Sure.
On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: > Commit 8c893aa3cd5 removed quotes that were required to detect > inline asm: > > check_insn armv5te qadd r0, r0, r0 > .../test.c:1:34: error: expected string literal in 'asm' > void foo(void){ __asm__ volatile(qadd r0, r0, r0); } > > The correct code is: > > void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } > --- a/configure > +++ b/configure > @@ -866,7 +866,7 @@ EOF > check_insn(){ > log check_insn "$@" > - check_inline_asm ${1}_inline "$2" > + check_inline_asm ${1}_inline "\"$2\"" > check_as ${1}_external "$2" > } This does not look like the correct fix to me. The required quotes should be part of the convenience function instead. Notice how calls to check_insn and check_inline_asm differ in the way they quote their arguments. There should be no need for this inconsistency. I'll look into it. Diego
On 6/7/2018 6:01 PM, Diego Biurrun wrote: > On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: >> Commit 8c893aa3cd5 removed quotes that were required to detect >> inline asm: >> >> check_insn armv5te qadd r0, r0, r0 >> .../test.c:1:34: error: expected string literal in 'asm' >> void foo(void){ __asm__ volatile(qadd r0, r0, r0); } >> >> The correct code is: >> >> void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } >> --- a/configure >> +++ b/configure >> @@ -866,7 +866,7 @@ EOF >> check_insn(){ >> log check_insn "$@" >> - check_inline_asm ${1}_inline "$2" >> + check_inline_asm ${1}_inline "\"$2\"" >> check_as ${1}_external "$2" >> } > > This does not look like the correct fix to me. The required quotes > should be part of the convenience function instead. Notice how calls > to check_insn and check_inline_asm differ in the way they quote their > arguments. There should be no need for this inconsistency. > > I'll look into it. Changing all the calls from check_insn name 'insn' to check_insn name '"insn"' would probably fix the check_inline_asm tests, but may break the check_as tests. I don't have an arm or qemu system to confirm the above, though. > > Diego > _______________________________________________ > libav-devel mailing list > libav-devel@libav.org > https://lists.libav.org/mailman/listinfo/libav-devel >
On Thu, Jun 07, 2018 at 11:05:26PM -0300, James Almer wrote: > On 6/7/2018 6:01 PM, Diego Biurrun wrote: > > On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: > >> Commit 8c893aa3cd5 removed quotes that were required to detect > >> inline asm: > >> > >> check_insn armv5te qadd r0, r0, r0 > >> .../test.c:1:34: error: expected string literal in 'asm' > >> void foo(void){ __asm__ volatile(qadd r0, r0, r0); } > >> > >> The correct code is: > >> > >> void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } > >> --- a/configure > >> +++ b/configure > >> @@ -866,7 +866,7 @@ EOF > >> check_insn(){ > >> log check_insn "$@" > >> - check_inline_asm ${1}_inline "$2" > >> + check_inline_asm ${1}_inline "\"$2\"" > >> check_as ${1}_external "$2" > >> } > > > > This does not look like the correct fix to me. The required quotes > > should be part of the convenience function instead. Notice how calls > > to check_insn and check_inline_asm differ in the way they quote their > > arguments. There should be no need for this inconsistency. > > > > I'll look into it. > > Changing all the calls from check_insn name 'insn' to check_insn name > '"insn"' would probably fix the check_inline_asm tests, but may break > the check_as tests. Complicating the function calls is not the right way to go. The helper function should take care of the required quoting and not rely on the callers to pass arguments in nested quotes. Diego
On Fri, 8 Jun 2018, Diego Biurrun wrote: > On Thu, Jun 07, 2018 at 11:05:26PM -0300, James Almer wrote: >> On 6/7/2018 6:01 PM, Diego Biurrun wrote: >> > On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: >> >> Commit 8c893aa3cd5 removed quotes that were required to detect >> >> inline asm: >> >> >> >> check_insn armv5te qadd r0, r0, r0 >> >> .../test.c:1:34: error: expected string literal in 'asm' >> >> void foo(void){ __asm__ volatile(qadd r0, r0, r0); } >> >> >> >> The correct code is: >> >> >> >> void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } >> >> --- a/configure >> >> +++ b/configure >> >> @@ -866,7 +866,7 @@ EOF >> >> check_insn(){ >> >> log check_insn "$@" >> >> - check_inline_asm ${1}_inline "$2" >> >> + check_inline_asm ${1}_inline "\"$2\"" >> >> check_as ${1}_external "$2" >> >> } >> > >> > This does not look like the correct fix to me. The required quotes >> > should be part of the convenience function instead. Notice how calls >> > to check_insn and check_inline_asm differ in the way they quote their >> > arguments. There should be no need for this inconsistency. >> > >> > I'll look into it. >> >> Changing all the calls from check_insn name 'insn' to check_insn name >> '"insn"' would probably fix the check_inline_asm tests, but may break >> the check_as tests. > > Complicating the function calls is not the right way to go. The helper > function should take care of the required quoting and not rely on the > callers to pass arguments in nested quotes. Ping; whoever is waiting for the other, please pick the thread up again. // Martin
On 25/06/2018 13:41, Martin Storsjö wrote: > On Fri, 8 Jun 2018, Diego Biurrun wrote: > >> On Thu, Jun 07, 2018 at 11:05:26PM -0300, James Almer wrote: >>> On 6/7/2018 6:01 PM, Diego Biurrun wrote: >>> > On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: >>> >> Commit 8c893aa3cd5 removed quotes that were required to detect >>> >> inline asm: >>> >> >>> >> check_insn armv5te qadd r0, r0, r0 >>> >> .../test.c:1:34: error: expected string literal in 'asm' >>> >> void foo(void){ __asm__ volatile(qadd r0, r0, r0); } >>> >> >>> >> The correct code is: >>> >> >>> >> void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } >>> >> --- a/configure >>> >> +++ b/configure >>> >> @@ -866,7 +866,7 @@ EOF >>> >> check_insn(){ >>> >> log check_insn "$@" >>> >> - check_inline_asm ${1}_inline "$2" >>> >> + check_inline_asm ${1}_inline "\"$2\"" >>> >> check_as ${1}_external "$2" >>> >> } >>> > > This does not look like the correct fix to me. The required quotes >>> > should be part of the convenience function instead. Notice how calls >>> > to check_insn and check_inline_asm differ in the way they quote their >>> > arguments. There should be no need for this inconsistency. >>> > > I'll look into it. >>> >>> Changing all the calls from check_insn name 'insn' to check_insn name >>> '"insn"' would probably fix the check_inline_asm tests, but may break >>> the check_as tests. >> >> Complicating the function calls is not the right way to go. The helper >> function should take care of the required quoting and not rely on the >> callers to pass arguments in nested quotes. > > Ping; whoever is waiting for the other, please pick the thread up again. > I'd wait another day and merge the proposed fix if nothing better materializes. lu
On Mon, Jun 25, 2018 at 02:41:30PM +0300, Martin Storsjö wrote: > On Fri, 8 Jun 2018, Diego Biurrun wrote: > > > On Thu, Jun 07, 2018 at 11:05:26PM -0300, James Almer wrote: > > > On 6/7/2018 6:01 PM, Diego Biurrun wrote: > > > > On Thu, Jun 07, 2018 at 03:03:21PM +0300, Martin Storsjö wrote: > > > >> Commit 8c893aa3cd5 removed quotes that were required to detect > > > >> inline asm: > > > >> > > > >> check_insn armv5te qadd r0, r0, r0 > > > >> .../test.c:1:34: error: expected string literal in 'asm' > > > >> void foo(void){ __asm__ volatile(qadd r0, r0, r0); } > > > >> > > > >> The correct code is: > > > >> > > > >> void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } > > > >> --- a/configure > > > >> +++ b/configure > > > >> @@ -866,7 +866,7 @@ EOF > > > >> check_insn(){ > > > >> log check_insn "$@" > > > >> - check_inline_asm ${1}_inline "$2" > > > >> + check_inline_asm ${1}_inline "\"$2\"" > > > >> check_as ${1}_external "$2" > > > >> } > > > > > This does not look like the correct fix to me. The required > > > quotes > > > > should be part of the convenience function instead. Notice how calls > > > > to check_insn and check_inline_asm differ in the way they quote their > > > > arguments. There should be no need for this inconsistency. > > > > > I'll look into it. > > > > > > Changing all the calls from check_insn name 'insn' to check_insn name > > > '"insn"' would probably fix the check_inline_asm tests, but may break > > > the check_as tests. > > > > Complicating the function calls is not the right way to go. The helper > > function should take care of the required quoting and not rely on the > > callers to pass arguments in nested quotes. > > Ping; whoever is waiting for the other, please pick the thread up again. Feel free to push this, I'll come up with a better fix later. Diego
diff --git a/configure b/configure index 5e79c0cec1..48e8536b07 100755 --- a/configure +++ b/configure @@ -866,7 +866,7 @@ EOF check_insn(){ log check_insn "$@" - check_inline_asm ${1}_inline "$2" + check_inline_asm ${1}_inline "\"$2\"" check_as ${1}_external "$2" }
From: John Cox <jc@kynesim.co.uk> Commit 8c893aa3cd5 removed quotes that were required to detect inline asm: check_insn armv5te qadd r0, r0, r0 .../test.c:1:34: error: expected string literal in 'asm' void foo(void){ __asm__ volatile(qadd r0, r0, r0); } The correct code is: void foo(void){ __asm__ volatile("qadd r0, r0, r0"); } Commit message written by Frank Liberato <liberato@chromium.org> --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)