[DSM-devel] dsme code cleanup, comments before merging?

[Ismo.Laitinen at nokia.com]

>-----Original Message-----
>From: dsm-devel-bounces at garage.maemo.org 
>[mailto:dsm-devel-bounces at garage.maemo.org] 
>Sent: 13 August, 2006 22:52
>To: liw at liw.iki.fi; dsm-devel at garage.maemo.org
>Subject: RE: [DSM-devel] dsme code cleanup, comments before merging?
>
>
>Hello,
>
>Could you Ismo and David take a moment and comment these if you
>haven't done it yet. 
>Anyhow, let's try to keep discussion in this mailing list :)

Hi all,

here are some initial comments. Sorry for the delay. I'll comment 
these now and the next mail a bit later.

All of the initial cleanups were okay but here's a question and a
comment.

>> * Changed everything to use dsme_log, instead of the DLOG_* macros.
>>   Having two logging methods in the same program isn't practical and
>>   can hopefully be avoided. Removed all use of DSME_LOG_ENABLED.

Regarding the logging.c. STI logging method is quite N770 specific
(or OMAP at least) and it relies on hardcoded channel number. Should
we do something for it?

>> * Added -g to the compilation flags in Makefile. This helps
>>   development; for production use, the binaries can be stripped upon
>>   installation.

Afaik, if env var DEBUG is defined -g was included already as then
C_DBGFLAGS were used.


>> 
>> Additional remarks for discussion:
>> 
>> * Error handling needs to become robust, no core dumping on broken
>>   connections (dsmetool), and at the very least, all memory
>>   allocations should be checked (strdup often isn't, now). Related
>>   to this, a decision on what to do upon these errors should happen:
>>   do we try to continue even though there is no memory? Or do we
>>   simply die, after trying to report the error? How critical is it
>>   for dsme to never die?

We need to come back to this. Basically in N770 dsme should never
die and if it does, hardware watchdog will reboot the device. Also,
on N770 dsme mallocs will practically never fail. If memory is really
running out the device gets so slow in kernel that dsme (or userspace
in general) doesn't get enough runtime to kick hardware watchdog.
But that's another topic :-)

But yes, there should be a consistent way to deal with errors. On
rather rare, but fatal errors (such as malloc fail) it's IMHO better 
to die and let hardware WD to reboot the device instead of trying to
recover. Just because dsme (and state management in general) should
be in a known state and recovering on severe problems might be 
tricky and unreliable. On smaller errors (mostly in the modules)
errors should be by-passed even with limited functionality.

BR, Ismo