Hello Denis,
On 09/09/2011 07:58 AM, Denis Kenzior wrote:
After reviewing this patch I got the impression that you were still
making things just way too complicated and not taking full advantage of
the push/pop semantics. So just for fun I wrote my own version in
plugins/mbpi.[ch] and reworked tools/lookup-apn to use that. The code
seems to be valgrind clean and can handle duplicate detection, bail out
early, etc.
Can you please take a look and suggest improvements? Once you think it
is good enough, I'd like you to rewrite this patch series to utilize
mbpi.c parser.
Thanks, I have just looked. I am fine with this approach too.
One of the ideas of the implementation in PATCHv6 is to have the naming of
tag_start() / tag_end() handlers to correspond to the actual tag names.
Can the same be achieved here? Can you please check PATCHv6 once again in order
to check that idea?
A few cosmetic issues:
- the naming of struct gsm_data, perhaps, parser_data or provider_info would
fit?
- gsm_start() could be broken down into smaller functions?
- Marcel wished the plugin naming "mobile-broadband-provider-info", even though
it's a very long name. Shall we stick to that? I am fine with both.
Perhaps, some diagnostic output should be added too, in case anybody will need
to troubleshoot it. For that, something like body_isprint() and ap_type_name()
would be needed.
Are you fine with these?
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
