Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Stipe Tolj]

Am 26.09.2016 19:47, schrieb Donald Jackson:

Fair point, ignore this patch in that case :)


np, we're happy that you're active... we still need to get SOMETHING, 
before refusing it ;)


Stipe

--
Best Regards,
Stipe Tolj

---
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/http://www.tolj.org/

stolj at kannel.org   st at tolj.org
---

Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Donald Jackson]

Fair point, ignore this patch in that case :)

On 26 September 2016 at 19:43, Stipe Tolj  wrote:

> Am 26.09.2016 19:38, schrieb Donald Jackson:
>
>> As I say this is semantics, its not entirely necessary.
>>
>> Usually in library functions they are compatible with each other, eg:
>>
>> SMPP_PDU *pdu = smpp_pdu_unpack(smpp_pdu_pack(data));
>>
>> Would work, but in Kannel's case this doesn't work as the one function
>> uses length and the other does not. So the third party user has to now
>> worry about PDU internals to get the functions to work together.
>>
>
> yeah, but looping in a "use-less" if condition for ever SMPP unpacking,
> JUST to make it sweet for the 3rd package is not justified IMO.
>
> I.e. here is what I do in smppbox's gw lib code for duplicating an SMPP
> PDU struct:
>
> SMPP_PDU *smpp_pdu_duplicate(Octstr *esme, SMPP_PDU *pdu)
> {
> SMPP_PDU *ret = NULL;
> Octstr *os, *os2;
>
> gw_assert(pdu != NULL);
>
> /*
>  * We use a kludge here, we pack the PDU
>  * then duplicate the packed data and unpack
>  * it again to a PDU structure.
>  */
> if ((os = SMPP_PDU_PACK(esme, pdu)) != NULL) {
> /* remove first 4 bytes, length indicator */
> os2 = octstr_copy(os, 4, octstr_len(os)-4);
> octstr_destroy(os);
> ret = SMPP_PDU_UNPACK(esme, os2);
> octstr_destroy(os2);
> }
>
> return ret;
>
> }
>
> Stipe
>
> --
> Best Regards,
> Stipe Tolj
>
> ---
> Düsseldorf, NRW, Germany
>
> Kannel Foundation tolj.org system architecture
> http://www.kannel.org/http://www.tolj.org/
>
> stolj at kannel.org   st at tolj.org
> ---
>
>


-- 
Donald Jackson

Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Stipe Tolj]

Am 26.09.2016 19:38, schrieb Donald Jackson:

As I say this is semantics, its not entirely necessary.

Usually in library functions they are compatible with each other, eg:

SMPP_PDU *pdu = smpp_pdu_unpack(smpp_pdu_pack(data));

Would work, but in Kannel's case this doesn't work as the one function
uses length and the other does not. So the third party user has to now
worry about PDU internals to get the functions to work together.


yeah, but looping in a "use-less" if condition for ever SMPP unpacking, 
JUST to make it sweet for the 3rd package is not justified IMO.


I.e. here is what I do in smppbox's gw lib code for duplicating an SMPP 
PDU struct:


SMPP_PDU *smpp_pdu_duplicate(Octstr *esme, SMPP_PDU *pdu)
{
SMPP_PDU *ret = NULL;
Octstr *os, *os2;

gw_assert(pdu != NULL);

/*
 * We use a kludge here, we pack the PDU
 * then duplicate the packed data and unpack
 * it again to a PDU structure.
 */
if ((os = SMPP_PDU_PACK(esme, pdu)) != NULL) {
/* remove first 4 bytes, length indicator */
os2 = octstr_copy(os, 4, octstr_len(os)-4);
octstr_destroy(os);
ret = SMPP_PDU_UNPACK(esme, os2);
octstr_destroy(os2);
}

return ret;
}

Stipe

--
Best Regards,
Stipe Tolj

---
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/http://www.tolj.org/

stolj at kannel.org   st at tolj.org
---

Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Donald Jackson]

As I say this is semantics, its not entirely necessary.

Usually in library functions they are compatible with each other, eg:

SMPP_PDU *pdu = smpp_pdu_unpack(smpp_pdu_pack(data));

Would work, but in Kannel's case this doesn't work as the one function uses
length and the other does not. So the third party user has to now worry
about PDU internals to get the functions to work together.



On 26 September 2016 at 19:31, Stipe Tolj  wrote:

> Am 24.09.2016 13:58, schrieb Rene Kluwen:
>
>> Sure, but why not change KSMPPD to use smpp_pdu_pack() function without
>> the length?
>>
>
> correct, IF this is something ksmppd needs/wants, then it needs to handle
> it on it's side.
>
> What is the purpose of NOT having the length being added?
>
> Stipe
>
> --
> Best Regards,
> Stipe Tolj
>
> ---
> Düsseldorf, NRW, Germany
>
> Kannel Foundation tolj.org system architecture
> http://www.kannel.org/http://www.tolj.org/
>
> stolj at kannel.org   st at tolj.org
> ---
>
>


-- 
Donald Jackson

Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Stipe Tolj]

Am 24.09.2016 13:58, schrieb Rene Kluwen:

Sure, but why not change KSMPPD to use smpp_pdu_pack() function without
the length?


correct, IF this is something ksmppd needs/wants, then it needs to 
handle it on it's side.


What is the purpose of NOT having the length being added?

Stipe

--
Best Regards,
Stipe Tolj

---
Düsseldorf, NRW, Germany

Kannel Foundation tolj.org system architecture
http://www.kannel.org/http://www.tolj.org/

stolj at kannel.org   st at tolj.org
---

Re: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Donald Jackson]

Yes that how it is currently, just semantically I think it would be better
to have all the PDU assembly/disassembly functions in one place.

It may also benefit others in future to have compatible pack/unpack
functions out the box.

On Saturday, 24 September 2016, Rene Kluwen  wrote:

> Sure, but why not change KSMPPD to use smpp_pdu_pack() function without
> the length?
>
>
>
> == Rene
>
>
>
>
>
> *Van:* devel [mailto:devel-boun...@kannel.org
> ] *Namens *Donald
> Jackson
> *Verzonden:* zaterdag 24 september 2016 12:07
> *Aan:* kannel_dev_mailinglist  >
> *Onderwerp:* [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c
>
>
>
> Hi all,
>
>
>
> Currently KSMPPD runs a patch against Kannel SVN trunk, I believe it would
> be better if we could just include this patch in the mainline Kannel as
> they don't have any negative implications.
>
>
>
> Currently the smpp_pdu_unpack() function takes data without length but the
> smpp_pdu_pack() function provides length so these two functions are not
> compatible with each other.  This simple patch just allows for
> compatibility convenience.
>
>
>
> Thanks,
>
> Donald
>


-- 
Donald Jackson
http://www.elite-sms-software.com
http://www.ddj.co.za
http://www.thearchitech.com
donald(a)thearchitech.com

RE: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

[Rene Kluwen]

Sure, but why not change KSMPPD to use smpp_pdu_pack() function without the 
length?

 

== Rene

 

 

Van: devel [mailto:devel-boun...@kannel.org] Namens Donald Jackson
Verzonden: zaterdag 24 september 2016 12:07
Aan: kannel_dev_mailinglist 
Onderwerp: [PATCH] KSMPPD (and other?) utility methods for smpp_pdu.c

 

Hi all, 

 

Currently KSMPPD runs a patch against Kannel SVN trunk, I believe it would be 
better if we could just include this patch in the mainline Kannel as they don't 
have any negative implications.

 

Currently the smpp_pdu_unpack() function takes data without length but the 
smpp_pdu_pack() function provides length so these two functions are not 
compatible with each other.  This simple patch just allows for compatibility 
convenience.

 

Thanks,

Donald