All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Boibessot <julien.boibessot@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] Add SMS Tools Server 3 package
Date: Thu, 16 Jan 2014 19:58:55 +0100	[thread overview]
Message-ID: <52D82BEF.2030209@free.fr> (raw)
In-Reply-To: <52D6D25B.3040603@zacarias.com.ar>

Hello Gustavo,

and thanks for the review !

On 01/15/2014 07:24 PM, Gustavo Zacarias wrote:
> On 01/08/2014 12:18 PM, julien.boibessot at free.fr wrote:
>
> Hi. Let's see...
>
>> +config BR2_PACKAGE_SMSTOOLS3_NB_MODEMS
>> +	string "Number of modems to support"
>> +	default 1
> What does this knob do in/to the code?

It (NUMBER_OF_MODEMS) is used in a very large number of loops of the
program and by default set to 64.

Doc says: "If you are running SMSTools3 in an embedded device, you can
use NUMBER_OF_MODEMS=1 to save memory."

Don't know why the author didn't choose to take dynamic command
parameters instead of static code define...

>
>> +
>> +config BR2_PACKAGE_SMSTOOLS3_START_AT_BOOT
>> +	bool "Start SMS Server Tools at system boot"
>> +	default y
> Ehhmm no, we don't make initscripts conditional that way, there are
> various hooks to get rid of it if someone doesn't want it that way or
> they run services their own way.

ok !

>
>> +
>> +endif
>> +
>> +comment "smstools3 needs a toolchain w/ wchar, threads"
>> +	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
>> diff --git a/package/smstools3/S50smsd b/package/smstools3/S50smsd
>> new file mode 100644
>> index 0000000..ded4189
>> --- /dev/null
>> +++ b/package/smstools3/S50smsd
>> @@ -0,0 +1,43 @@
>> +#!/bin/sh
>> +#
>> +# Starts the SMS Server Tools 3
>> +#
>> +
>> +NAME=smsd
>> +DAEMON=/usr/local/bin/$NAME
> /usr/local? Why?

Well... no particular reason :-)
/usr would be ok ?

>
>> +ifeq ($(BR2_PACKAGE_SMSTOOLS3_START_AT_BOOT),y)
>> +SMSTOOLS3_INIT_SCRIPT_NAME = S50smsd
>> +else
>> +SMSTOOLS3_INIT_SCRIPT_NAME = 50smsd
>> +endif
> Please use SMSTOOLS3_INSTALL_INIT_SYSV for the script.
> See docs/manual/adding-packages-generic.txt for the reason.

ok !

>
> Otherwise looking good in general without testing it though.

Thanks !
I will send a corrected version tomorrow.

Regards,
Julien

  reply	other threads:[~2014-01-16 18:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-08 15:18 [Buildroot] [PATCH] Add SMS Tools Server 3 package julien.boibessot at free.fr
2014-01-15 18:14 ` Julien Boibessot
2014-01-15 18:24 ` Gustavo Zacarias
2014-01-16 18:58   ` Julien Boibessot [this message]
2014-01-17 11:39     ` Gustavo Zacarias

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52D82BEF.2030209@free.fr \
    --to=julien.boibessot@free.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.