Hello :)
Now I've implemented the TP-VPF-ENHANCED. To achieve that I added some goto-stuff - i hope that's ok. I think it makes sense here.
It's again untested, but compiles fine. I'm working at the moment on a way to test SMS-PDU decoding.
If that's easier you can simply merge the branch tp-vp from git://github.com/stefreak/openbsc.git :)
The patch is attached.
Greetings, Steffen
On Tuesday 17 November 2009 19:30:39 Steffen 'stefreak' Neubauer wrote:
Hello :)
Now I've implemented the TP-VPF-ENHANCED. To achieve that I added some goto-stuff - i hope that's ok. I think it makes sense here.
I think it is one of the cases where a function call makes more sense?
On Tue, 17 Nov 2009 19:44:18 +0100 Holger Freyther zecke@selfish.org wrote:
I think it is one of the cases where a function call makes more sense?
Hmm, really? All this stuff is only needed (as far as i know, of course ;)) for the validity-period decoding - nowhere else. Would you really put all those small VPF-implementations into their own functions? At the moment only the service centre time stamp decoding has it's own function because it's needed elsewhere. Isn't it clear enough with 'goto'?
Greetings, Steffen
On Tuesday 17 November 2009 20:02:13 Steffen 'stefreak' Neubauer wrote:
On Tue, 17 Nov 2009 19:44:18 +0100
Holger Freyther zecke@selfish.org wrote:
I think it is one of the cases where a function call makes more sense?
Hmm, really? All this stuff is only needed (as far as i know, of course ;)) for the validity-period decoding - nowhere else. Would you really put all those small VPF-implementations into their own functions? At the moment only the service centre time stamp decoding has it's own function because it's needed elsewhere. Isn't it clear enough with 'goto'?
Yes, I would put all these into static methods... the compiler will inline them for you. The problem with this amount of gotos is it is really hard to understand which ways you are jumping. E.g. within "enhanced" you will do a backward jump. The problem with that it is very hard to make sure that your jumping will end... if/else if and function calls are much easier to read.
holger
On Wed, 18 Nov 2009 09:43:48 +0100 Holger Freyther zecke@selfish.org wrote:
Yes, I would put all these into static methods... the compiler will inline them for you. The problem with this amount of gotos is it is really hard to understand which ways you are jumping. E.g. within "enhanced" you will do a backward jump. The problem with that it is very hard to make sure that your jumping will end... if/else if and function calls are much easier to read.
Okay, Thank You for your comments! The new patch is attached.
Again, If that's easier you can simply merge my tp-vp branch on github.
Thank You and Greetings, Steffen
Thanks,
applied your patch for VPF_ENHANCED.
Steffen 'stefreak' Neubauer schreef:
Hello :)
Now I've implemented the TP-VPF-ENHANCED. To achieve that I added some goto-stuff - i hope that's ok. I think it makes sense here.
Goto seems here not necessary, I think. Why don't you just put the code in each case: instead of jumping to a tag and go from there? Set the return value retval or something and when you break out the switch statement you return the retval. But that's just my humble opinion :)