I wondered if stack size might be a concern, since the header is moderately large (36 bytes per buffer desc, plus 32 for header), but PTPSession seems to get a pretty generous size: 0x1000 on a540, 0x1800 on g7x and the stock PTP handlers are pretty complicated.
I missed a couple of initialisations when sections are not being sent - updated patch attached.If you're happy with this I'll check it in.
Index: core/gui.c===================================================================--- core/gui.c (revision 5293)+++ core/gui.c (working copy)@@ -2401,7 +2401,7 @@ } //--------------------------------------------------------------------static gui_handler *gui_mode=0; // current gui mode. pointer to gui_handler structure+static gui_handler *gui_mode = &altGuiHandler; // current gui mode. pointer to gui_handler structure static int gui_osd_need_restore = 0; // Set when screen needs to be erase and redrawn static int gui_mode_need_redraw = 0; // Set if current mode needs to redraw itself
Spytask dereferences a NULL pointer during its startup.gui_init() -> gui_set_mode() -> the gui_mode pointer is NULL initiallyDoes the following patch look good enough to fix this?Code: [Select]Index: core/gui.c===================================================================--- core/gui.c (revision 5293)+++ core/gui.c (working copy)@@ -2401,7 +2401,7 @@ } //--------------------------------------------------------------------static gui_handler *gui_mode=0; // current gui mode. pointer to gui_handler structure+static gui_handler *gui_mode = &altGuiHandler; // current gui mode. pointer to gui_handler structure
Index: core/gui.c===================================================================--- core/gui.c (revision 5293)+++ core/gui.c (working copy)@@ -2401,7 +2401,7 @@ } //--------------------------------------------------------------------static gui_handler *gui_mode=0; // current gui mode. pointer to gui_handler structure+static gui_handler *gui_mode = &altGuiHandler; // current gui mode. pointer to gui_handler structure
I think I'd use defaultGuiHandler
Or adjust gui_set_mode to deal with null old_mode.
Quote from: reyalp on 19 / November / 2019, 21:45:36I think I'd use defaultGuiHandlerThat would not work very well because gui_init() would fail to initialize the gui mode.
QuoteOr adjust gui_set_mode to deal with null old_mode.I tried that first and failed due to my insufficient knowledge about the inner workings of the CHDK GUI. Also, any change there would lead to an increase in core size.Using &altGuiHandler as initial value of that pointer would mean no increase in core size. This does not mean CHDK would start in ALT mode, that struct would only provide values that (IMO) allow a successful transition to default GUI mode.I don't think I see any related glitches on the M100.Currently, the first few words at address zero are used as initial value when members of that struct are accessed.
Quote from: srsa_4c on 20 / November / 2019, 12:16:57Quote from: reyalp on 19 / November / 2019, 21:45:36I think I'd use defaultGuiHandlerThat would not work very well because gui_init() would fail to initialize the gui mode.Yeah, that makes sense, I didn't think through the if mode == current mode logic.QuoteQuoteOr adjust gui_set_mode to deal with null old_mode.I tried that first and failed due to my insufficient knowledge about the inner workings of the CHDK GUI. Also, any change there would lead to an increase in core size.Using &altGuiHandler as initial value of that pointer would mean no increase in core size. This does not mean CHDK would start in ALT mode, that struct would only provide values that (IMO) allow a successful transition to default GUI mode.I don't think I see any related glitches on the M100.Currently, the first few words at address zero are used as initial value when members of that struct are accessed.If it works, that's good enough for me. It should be better than the current behavior. My initial reaction was that I'd prefer not to have it initialized in an inconsistent state (that is gui_mode is "alt" but various other gui and alt state variables initialized as non-alt).FWIW, I'm not at all concerned about increasing core size by a few hundreds of bytes if it makes things clearer or easier to maintain.One thing that might thinking about more is if any of the gui_mode stuff might be accessed in other tasks before gui_init is run, since that happens quite late after the filesystem is ready and CHDK settings have been initialized.The fact that your solution works on M100 suggests it isn't on that camera. One possible exception is cameras with touch screen support (n, ixus310, ixus240) which reference it in the touch task. I doubt your change will hurt those, but it would probably be better to have chdk_process_touch immediately return if gui_init hasn't run yet. I don't think this needs to be done to check your change in.Startup scripts are run from spytask after gui_init, so they shouldn't be a concern.
I can't see any other cases - am I missing anything?
Quote from: philmoz on 20 / November / 2019, 15:03:07I can't see any other cases - am I missing anything?Perhaps stating the obvious, but I assume the ones srsa_4c is hitting is the ones gui_set_mode where it references old_mode->... on 2474 and 2477It seems like those could just be skipped if old_mode is null, though I'm not immediately sure whether redraw / restore stuff should be called on the first call.gui_set_mode also returns old_mode, which would be NULL in this case, but the value isn't used in gui_init.
Quote from: reyalp on 20 / November / 2019, 15:23:42Quote from: philmoz on 20 / November / 2019, 15:03:07I can't see any other cases - am I missing anything?Perhaps stating the obvious, but I assume the ones srsa_4c is hitting is the ones gui_set_mode where it references old_mode->... on 2474 and 2477It seems like those could just be skipped if old_mode is null, though I'm not immediately sure whether redraw / restore stuff should be called on the first call.gui_set_mode also returns old_mode, which would be NULL in this case, but the value isn't used in gui_init.Good point, missed that case.Setting a default for gui_mode makes more sense now - I'm not fully comfortable with the idea of using the alt mode handler though.Perhaps a dummy handler struct for startup until gui_init is called might work.Phil.
Started by andre117 General Discussion and Assistance
Started by Bernd R General Discussion and Assistance
Started by quid « 1 2 » General Discussion and Assistance
Started by zell « 1 2 » General Discussion and Assistance
Started by srsa_4c « 1 2 » General Discussion and Assistance
