RE: [PATCH] (sqlbox) sqlbox.patch

Some comments:

- configure SHOULD be included in the patch. Because "nobody" uses autoconf
to build their configure itself.
This is way different from .o files... because they are not included in the
repository.

- Yes, a seperate build tree for sqlbox might be cleaner. But we can only do
that if we work towards an unpatched Kannel that we can build against.
Meaning all patches that are in sqlbox.patch should go in CVS *or* sqlbox
should work around it in another way (e.g. in the case of cfg.def).

- The quotes issue is indeed a matter of Kannel and not of sqlbox. Yes, it
should be included in Kannel CVS.
Stipe: Try building plain Kannel (without sqlbox patch) with MySQL support
on the kannel.org host.
You will notice that it won't want to 'make'.

- The bearerbox-is-sqlbox directive is to let smsbox know that it should
connect to the sqlbox-port in stead of the bearerbox-port.
That is all there is to it.

- Maybe you should post the source codes of your smppbox here so we can all
see how you did it ;)


-----Original Message-----
From: [email protected] [mailto:[email protected]]On
Behalf Of Stipe Tolj
Sent: woensdag 14 juni 2006 0:56
To: Mi Reflejo
Cc: [email protected]
Subject: Re: [PATCH] (sqlbox) sqlbox.patch


Mi Reflejo wrote:

> New sqlbox.patch for sqlbox module according to the last gateway update.

Hi Martin,

thanks a lot for the submission. We appritiate your work on Kannel, and I
have
to admit that this is a good way to go...

Now, some critics anyway (even while Rene is more in charge for sqlbox ;)

* Please don't include configure itself in the patchset. It's "generated"
code
and hence, only configure.in belongs to the patchset. (It's like if we would
add
.o files to the CVS ;)

* The Makefile.in change is necessary to let Kannel core build sqlbox. This
can
be moved out of Kannel core in order to allow sqlbox (as own add-on module)
to
build itself again Kannel core (gwlib, gw) libs... I could forward you my
add-on
   sceleton configure.in and Makefile.in to use for such purposes. You could
go
then and addopt it for sqlbox?

* Why is
diff -u -r1.166 configure.in
--- configure.in.orig   2006-06-13 15:27:52.000000000 -0600
+++ configure.in        2006-06-13 15:27:15.000000000 -0600
@@ -936,14 +936,14 @@
          dnl mysql-4.x style
          MYSQL_LIBS=""
          if $MYSQL_CONFIG --libs_r &>/dev/null ; then
-            MYSQL_LIBS=`$MYSQL_CONFIG --libs_r`
+            MYSQL_LIBS=`$MYSQL_CONFIG --libs_r | sed -e "s/\x27//g"`
              AC_MSG_CHECKING([mysql reentrant libs])
              AC_MSG_RESULT([$MYSQL_LIBS])
              AC_CHECK_LIB(mysqlclient_r, mysql_init, [ LIBS="$LIBS
$MYSQL_LIBS" ],
                  [ MYSQL_LIBS="" ], [ $MYSQL_LIBS ])
          fi
          if test -z "$MYSQL_LIBS" ; then
-            MYSQL_LIBS=`$MYSQL_CONFIG --libs`
+            MYSQL_LIBS=`$MYSQL_CONFIG --libs | sed -e "s/\x27//g"`
              AC_MSG_CHECKING([mysql libs])
              AC_MSG_RESULT([$MYSQL_LIBS])
              AC_CHECK_LIB(mysqlclient, mysql_init, [ LIBS="$LIBS
$MYSQL_LIBS" ],

necessary? Was this the issue with the ' signs?
If yes, this is of general scope and should go to gateway core itself.

* Why is config directive 'bearerbox-is-sqlbox' necessary for smsbox.c? I
don't
see a usage of the added int bearerbox_is_sqlbox in the smsbox.c patchset?

Does this mean sqlbox acts towards a "patched" smsbox like a bearerbox? Like
this:

   user <-HTTP-> smsbox <-> sqlbox <-> bearerbox

?

* sqlbox should have it's own module gwlib/sqlbox-cfg.def and call the
cfg_add_hooks() routine I proposed to "load" the semantic checks for the new
config group.


Hope I wasn't to critic ;)... actually I like the sqlbox itself, but would
suggest Rene and you to mangle it the way I used for smppbox, where the
module
should be as much isolated from patching the core as possible. In the end I
came
up with the single (still to vote) patch for the cfg_add_hook() patch for
gwlib/cfg.[ch], and everything else would be in foobar module dir itself....
works perfectly and smooth.

Stipe

-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany

tolj.org system architecture      Kannel Software Foundation (KSF)
http://www.tolj.org/              http://www.kannel.org/

mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------