Re: [Xen-devel] [PATCH] x86/HVM: make explicit that hvm_print_line() does output only

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.17 at 17:52, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 06/12/17 16:27, Jan Beulich wrote:
>> On input "c" being 0xff should already have the effect of bailing early
>> (due to the isprint()), but let's rather make this explicit. Also
>> convert the BUG_ON() to an ASSERT() (nothing fatal happens in the
>> function if this is violated), at the same time extending what is being
>> checked.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -544,10 +544,14 @@ static int hvm_print_line(
>>      struct domain *cd = current->domain;
>>      char c = *val;
>>  
>> -    BUG_ON(bytes != 1);
>> +    ASSERT(bytes == 1 || port == 0xe9);
>>  
>> -    /* Accept only printable characters, newline, and horizontal tab. */
>> -    if ( !isprint(c) && (c != '\n') && (c != '\t') )
>> +    /*
>> +     * Ignore any input requests and accept only printable characters,
>> +     * newline, and horizontal tab.
>> +     */
>> +    if ( dir != IOREQ_WRITE ||
>> +         (!isprint(c) && (c != '\n') && (c != '\t')) )
>>          return X86EMUL_OKAY;
> 
> Given that there is no functionality on the read side, it should be
> explicitly terminated with ~0 rather than ignored.

This is already being taken care of in the caller (hvm_portio_read()).

> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Let me know if this stands without making further adjustments to
the patch.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel