Re: [Xen-devel] [PATCH v5 03/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.09.16 at 16:23, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 09/23/2016 05:18 AM, Jan Beulich wrote:
>>
>>> @@ -36,18 +37,25 @@ ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl 
>>> iasl
>>>  mk_dsdt: mk_dsdt.c
>>>     $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
>>>  
>>> -dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl mk_dsdt
>>> +ifeq ($(GPL),y)
>>> +dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl gpl/mk_dsdt_gpl.sh 
>>> mk_dsdt
>>>     awk 'NR > 1 {print s} {s=$$0}' $< > $@.$(TMP_SUFFIX)
>>> +   # Strip license comment
>>> +   sed -i '1,/\*\//{/\/\*/,/\*\//d}' $@.$(TMP_SUFFIX)
>>> +   ./gpl/mk_dsdt_gpl.sh >> $@.$(TMP_SUFFIX)
>> I don't think the leading ./ is necessary here, ...
>>
>>>     cat dsdt_acpi_info.asl >> $@.$(TMP_SUFFIX)
>>>     ./mk_dsdt --debug=$(debug) --dm-version qemu-xen >> $@.$(TMP_SUFFIX)
>> ... other than e.g. here.
> 
> What is the difference between the two?

The latter would then lack any directory components, and hence
would become subject to lookup through $PATH, which would fail
if . is not included in it (and which would possibly do the wrong
thing even if it was).

>>> --- /dev/null
>>> +++ b/tools/firmware/hvmloader/acpi/gpl/mk_dsdt_gpl.sh
>>> @@ -0,0 +1,110 @@
>>> +# This program is free software; you can redistribute it and/or modify it
>>> +# under the terms and conditions of the GNU General Public License,
>>> +# version 2, as published by the Free Software Foundation.
>>> +#
>>> +# This program is distributed in the hope it will be useful, but WITHOUT
>>> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> +# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>> +# more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License along 
>>> with
>>> +# this program; If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +
>>> +printf "\n    /* Beginning of GPL-only code */\n\n"
>>> +
>>> +printf "    /* _S3 and _S4 are in separate SSDTs */\n"
>>> +printf "    Name (\_S5, Package (0x04) {\n"
>>> +printf "        0x00,  /* PM1a_CNT.SLP_TYP */\n"
>>> +printf "        0x00,  /* PM1b_CNT.SLP_TYP */\n"
>>> +printf "        0x00,  /* reserved */\n"
>>> +printf "        0x00   /* reserved */\n"
>>> +printf "    })\n"
>>> +
>>> +printf "    Name(PICD, 0)\n"
>>> +printf "    Method(_PIC, 1) {\n"
>>> +printf "        Store(Arg0, PICD)\n"
>>> +printf "    }\n"
>> Wouldn't this be better readable with "echo", avoiding all the \n
>> instances? Actually this seems to even apply to most if not
>> everything further down, as so far I didn't spot a case where
>> you actually pass anything other than just a format string.
> 
> Format string is the only reason. There are a couple of instances where
> output is formatted and I felt that having both echo and printf would
> feel inconsistent.

Hmm, I think avoiding the many \n would warrant this slight
inconsistency.

>>> +# PCI-ISA link definitions
>>> +
>>> +# BUFA: List of ISA IRQs available for linking to PCI INTx.
>>> +printf "    Scope ( \_SB.PCI0 )  {\n"
>>> +printf "        Name ( BUFA, ResourceTemplate() { IRQ(Level, ActiveLow, 
>>> Shared) { 5, 10, 11 } } )\n"
>>> +# BUFB: IRQ descriptor for returning from link-device _CRS methods.
>>> +printf "        Name ( BUFB, Buffer() { 0x23, 0x00, 0x00, 0x18, 0x79, 0 } 
>>> )\n"
>>> +printf "        CreateWordField ( BUFB, 0x01, IRQV )\n"
>>> +
>>> +links=(A B C D)
>> Is this portable?
> 
> I heard that associative arrays sometimes have portability issues but
> this is a regular one. FWIW this runs on a 2009 Fedora12.
> 
> Would 'declare -a links' help?

I don't know - honestly I'm not a shell script expert. All I can say is
that I wasn't able to find the construct above e.g. in SUS v6.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel