>>> On 13.05.16 at 05:39, <quan...@intel.com> wrote:
> On May 10, 2016 5:25 PM, Jan Beulich <jbeul...@suse.com> wrote:
>> >>> On 06.05.16 at 10:54, <quan...@intel.com> wrote:
>> > --- a/xen/arch/x86/acpi/power.c
>> > +++ b/xen/arch/x86/acpi/power.c
>> > static int device_power_down(void)
>> > {
>> > - console_suspend();
>> > + if ( console_suspend() )
>> > + return TYPE_CONSOLE;
>>
>> This (together with the resume side) makes me guess that the use of TYPE_ as
>> a prefix confused not just me, but also you:
>
> Yes, this is really not a good prefix, and probably pretty bad to use
> 'ERROR_'.
> What about 'PRIOR_'? then I also need to adjust device_power_up() as ...
What about SAVED_?
>> > - time_suspend();
>> > + if ( time_suspend() )
>> > + return TYPE_TIME;
>> >
>> > - i8259A_suspend();
>> > + if ( i8259A_suspend() )
>> > + return TYPE_I8259A;
>> >
>> > + /* ioapic_suspend should never fail */
>> > ioapic_suspend();
>>
>> The comment is bogus: "should" means it can in theory. Yet the function
>> having void return type means it just cannot fail.
>>
>
> I'll use 'cannot', instead of 'should'.
> Another question, I check the code again, and the rest of the functions
> (console_suspend/ time_suspend/ i8259A_suspend / ioapic_suspend /
> lapic_suspend ), in device_power_down(), always returned '0'.
> Maybe I need to fix these functions annotation from 'int' to 'void', and
> then I can add a comment on the device_power_down().
Please don't. Generally the possibility of failure exists, and hence if
functions have already been written to account for that, we shouldn't
strip that capability out.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
