diff mbox

[v3,1/2] fuzz/x86emul: avoid race in link farm rune

Message ID 20170220120535.25343-2-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu Feb. 20, 2017, 12:05 p.m. UTC
Several `ln -sf` can race with each other and cause error like:

14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory

Provide dedicated targets for soft-linking directories.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: use pattern matching rule
---
 tools/fuzz/x86_instruction_emulator/Makefile | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Jan Beulich Feb. 20, 2017, 12:37 p.m. UTC | #1
>>> On 20.02.17 at 13:05, <wei.liu2@citrix.com> wrote:
> Several `ln -sf` can race with each other and cause error like:
> 
> 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory
> 
> Provide dedicated targets for soft-linking directories.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: use pattern matching rule
> ---
>  tools/fuzz/x86_instruction_emulator/Makefile | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/fuzz/x86_instruction_emulator/Makefile 
> b/tools/fuzz/x86_instruction_emulator/Makefile
> index fede7e9afd..b0ea31b2b0 100644
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -8,11 +8,15 @@ else
>  x86-instruction-emulator-fuzzer-all:
>  endif
>  
> -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> -	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> +x86_emulate:
> +	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/arch/x86/$@ .

For consistency with the ln at the very bottom (patch context) you
may want to omit the . here.

> -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> -	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate

So why don't you use a pattern rule here?

> +asm:
> +	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 $@
> +
> +asm/%: asm ;

Is the semicolon needed here? You don't have one above ...

Jan

>  x86_emulate.c x86_emulate.h: %:
>  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
Wei Liu Feb. 20, 2017, 2:05 p.m. UTC | #2
On Mon, Feb 20, 2017 at 05:37:56AM -0700, Jan Beulich wrote:
> >>> On 20.02.17 at 13:05, <wei.liu2@citrix.com> wrote:
> > Several `ln -sf` can race with each other and cause error like:
> > 
> > 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory
> > 
> > Provide dedicated targets for soft-linking directories.
> > 
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > v3: use pattern matching rule
> > ---
> >  tools/fuzz/x86_instruction_emulator/Makefile | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/fuzz/x86_instruction_emulator/Makefile 
> > b/tools/fuzz/x86_instruction_emulator/Makefile
> > index fede7e9afd..b0ea31b2b0 100644
> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> > @@ -8,11 +8,15 @@ else
> >  x86-instruction-emulator-fuzzer-all:
> >  endif
> >  
> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> > -	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> > +x86_emulate:
> > +	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/arch/x86/$@ .
> 
> For consistency with the ln at the very bottom (patch context) you
> may want to omit the . here.
> 

Sure.

> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> > -	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> 
> So why don't you use a pattern rule here?
> 

Forgot this place. Will fix.

> > +asm:
> > +	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 $@
> > +
> > +asm/%: asm ;
> 
> Is the semicolon needed here? You don't have one above ...
> 

Yes, this is needed; otherwise make complains not knowing how to make
the target.  When I change x86_emulate.[ch] to use patter rule, I will
need to add a semicolon as well.

From Make manual for empty recipes:

Empty recipes can also be used to avoid errors for targets that will be
created as a side-effect of another recipe: if the target does not exist
the empty recipe ensures that make won’t complain that it doesn't know
how to build the target, and make will assume the target is out of date.

Wei.

> Jan
> 
> >  x86_emulate.c x86_emulate.h: %:
> >  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
> 
>
Jan Beulich Feb. 20, 2017, 2:28 p.m. UTC | #3
>>> On 20.02.17 at 15:05, <wei.liu2@citrix.com> wrote:
> On Mon, Feb 20, 2017 at 05:37:56AM -0700, Jan Beulich wrote:
>> >>> On 20.02.17 at 13:05, <wei.liu2@citrix.com> wrote:
>> > +asm:
>> > +	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 $@
>> > +
>> > +asm/%: asm ;
>> 
>> Is the semicolon needed here? You don't have one above ...
>> 
> 
> Yes, this is needed; otherwise make complains not knowing how to make
> the target.  When I change x86_emulate.[ch] to use patter rule, I will
> need to add a semicolon as well.
> 
> From Make manual for empty recipes:
> 
> Empty recipes can also be used to avoid errors for targets that will be
> created as a side-effect of another recipe: if the target does not exist
> the empty recipe ensures that make won’t complain that it doesn't know
> how to build the target, and make will assume the target is out of date.

Oh, yes, of course.

Jan
diff mbox

Patch

diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index fede7e9afd..b0ea31b2b0 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -8,11 +8,15 @@  else
 x86-instruction-emulator-fuzzer-all:
 endif
 
-x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
-	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
+x86_emulate:
+	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/arch/x86/$@ .
 
-asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
-	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
+x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
+
+asm:
+	[ -L $@ ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 $@
+
+asm/%: asm ;
 
 x86_emulate.c x86_emulate.h: %:
 	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*