Re: [PATCH 2/7] tboot: Add return values for tboot_sleep

From: Konrad Rzeszutek Wilk
Date: Fri Feb 03 2012 - 15:08:21 EST


On Thu, Jan 26, 2012 at 12:16:39AM +0000, Cihula, Joseph wrote:
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> > Sent: Thursday, January 12, 2012 12:23 PM
> eturn values for tboot_sleep
> >
> > On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> > > > ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you
> > are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its
> > errors through tboot_sleep().
> > >
> > > Hey Joe,
> > >
> > > Thanks for looking at the patches.
> > >
> > > Right now [with this patch applied] the code looks as so:
> > >
> > > 297
> > > 298 tboot_shutdown(acpi_shutdown_map[sleep_state]);
> > > 299 return 0;
> > > 300 }
> > >
> > >
> > > If we do make tboot_shutdown() return an int, there will be a need to
> > > modify other callers: native_machine_emergency_restart,
> > > native_machine_halt, native_machine_power_off, and native_play_dead as well.
> > >
> > > Perhaps that could be done in another patch and leave this one as is
> > > where the 'tboot_sleep' would just return 0 instead of 'return
> > > tboot_shutdown(...)' ?
> > >
> > > Perhaps another way to do this is to extract the common code of
> > > tboot_sleep and tboot_shutdown?
>
> I'd be happier to see the callers above changed to handle a tboot_shutdown() that returned an error than splitting this up and only checking the error in one path.

OK, so something like this then: