Bug 374771 - Review Request: smstools - Tools for send and receive short messages through GSM modems or mobile phones
Review Request: smstools - Tools for send and receive short messages through ...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
: Reopened
: 235637 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-10 10:41 EST by Marek Mahut
Modified: 2015-12-03 20:06 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-03 20:06:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marek Mahut 2007-11-10 10:41:58 EST
Spec URL: http://mmahut.fedorapeople.org/reviews/smstools/smstools.spec
SRPM URL: http://mmahut.fedorapeople.org/reviews/smstools/smstools-2.2.18-1.src.rpm
Mock: http://mmahut.fedorapeople.org/reviews/smstools/mock/
Description: The SMS Server Tools send and receive short messages
through GSM modems or mobile phones. You can send short messages
by simply storing text files into a special spool directory.
Comment 1 Lubomir Kundrak 2007-11-10 10:44:35 EST
Thanks for the package, taking this for a review.
Comment 2 Lubomir Kundrak 2007-11-10 11:05:52 EST
Marek: The package looks superb. Just a few minor notes before I can approve it:

Summary is not correct american english.
I believe the License is GPLv2+, not GPLv2

RPMlint output:

Lot of

smstools.i386: W: wrong-file-end-of-line-encoding
/usr/share/doc/smstools-2.2.18/index.html

and

smstools.i386: W: wrong-file-end-of-line-encoding
/usr/share/doc/smstools-2.2.18/license.html

you probably need some recode/iconv/dos2unix magic

smstools.src:22: W: setup-not-quiet

just add -q to %setup

smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 1)

safe to ignore
Comment 3 Ville Skyttä 2007-11-10 11:55:44 EST
We already have smstools under review in bug 235637.  The submitter hasn't
however been at all responsive to my comments in ~7 months so I think it's
better to continue here.

It would be useful to check if any of my findings in that bug apply here too
though, and check if it makes more sense to package the original 2.x version or
the enhanced one from http://smstools3.kekekasvi.com/ (which is what was
submitted in bug 235637).
Comment 4 Ville Skyttä 2007-11-10 11:57:34 EST
At least the /usr/bin/sendsms conflict with gnokii from bug 235637 appears to
apply to this package too.
Comment 5 Marek Mahut 2007-11-10 12:55:34 EST
Hello Ville, Hello Lubomir,

So, I've modified the spec file from Andreas.

  http://mmahut.fedorapeople.org/reviews/smstools/3.0.6/

I've renamed the script to smssend, which is better from my point of view,
because other smstools scripts starts with "sms*" too.

I will try to implement the feature of running with no-root user later, I will
check with upstream if it's possible. But for now, running as root is enough for
me (we will use this package in my company).

Let me know in case of any other suggestions.
Comment 6 Lubomir Kundrak 2007-11-12 11:28:34 EST
Sooo, Marek:

In my opinion you shouldn't change modes in threee different places; (ugly
chmods in %prep, install -m %install and %attr in %files).

And RPMlint:

smstools.i386: W: file-not-utf8
/usr/share/doc/smstools-3.0.6/manual/softwarecomp.html
...

Alreay discussed that, not a problem.

smstools.i386: W: file-not-utf8
/usr/share/doc/smstools-3.0.6/examples/send_sms_unicode.sms

UNICODE example, whouldn't make sense if was utf8

smstools.i386: E: non-standard-dir-perm /var/spool/sms/failed 0750
smstools.i386: E: non-standard-dir-perm /var/spool/sms/checked 0750
smstools.i386: E: non-standard-dir-perm /var/spool/sms/outgoing 0750
smstools.i386: E: non-readable /etc/smsd.conf 0600
smstools.i386: E: non-standard-dir-perm /var/spool/sms/sent 0750
smstools.i386: E: non-standard-dir-perm /var/spool/sms/incoming 0750
smstools.i386: E: non-standard-dir-perm /var/spool/sms 0750

I think this is all intended

smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools
smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools
smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools

Using /var/lock/subsys/smsd for a subsys lock instead of
/var/lock/subsys/smstools would make more sense in my opinion.

smstools.i386: W: incoherent-init-script-name smsd

Perfectly ok

smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 66, tab: line 1)

Run expand on the spec.
Comment 7 Marek Mahut 2007-11-12 14:17:56 EST
(In reply to comment #6)
> Sooo, Marek:
> 
> In my opinion you shouldn't change modes in threee different places; (ugly
> chmods in %prep, install -m %install and %attr in %files).

Done, let me know if it's ok.

> And RPMlint:
> 
> smstools.i386: W: file-not-utf8
> /usr/share/doc/smstools-3.0.6/manual/softwarecomp.html
> ...
> 
> Alreay discussed that, not a problem.
> 
> smstools.i386: W: file-not-utf8
> /usr/share/doc/smstools-3.0.6/examples/send_sms_unicode.sms
> 
> UNICODE example, whouldn't make sense if was utf8
> 
> smstools.i386: E: non-standard-dir-perm /var/spool/sms/failed 0750
> smstools.i386: E: non-standard-dir-perm /var/spool/sms/checked 0750
> smstools.i386: E: non-standard-dir-perm /var/spool/sms/outgoing 0750
> smstools.i386: E: non-readable /etc/smsd.conf 0600
> smstools.i386: E: non-standard-dir-perm /var/spool/sms/sent 0750
> smstools.i386: E: non-standard-dir-perm /var/spool/sms/incoming 0750
> smstools.i386: E: non-standard-dir-perm /var/spool/sms 0750
> 
> I think this is all intended

Indeed.

> smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools
> smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools
> smstools.i386: E: incoherent-subsys /etc/rc.d/init.d/smsd smstools
> 
> Using /var/lock/subsys/smsd for a subsys lock instead of
> /var/lock/subsys/smstools would make more sense in my opinion.

Done.

> smstools.i386: W: incoherent-init-script-name smsd
> 
> Perfectly ok
> 
> smstools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 66, tab: line 1)
> 
> Run expand on the spec.

Done.

     Updated spec file: http://mmahut.fedorapeople.org/reviews/smstools/3.0.6/


Comment 8 Marek Mahut 2007-11-12 14:27:46 EST
New version is here: http://mmahut.fedorapeople.org/reviews/smstools/3.0.10/
Comment 9 Lubomir Kundrak 2007-11-12 14:37:18 EST
A few other things; Please fix the source URL, and bump to latest upstream version.
Comment 10 Marek Mahut 2007-11-12 14:41:25 EST
Done.
Comment 11 Lubomir Kundrak 2007-11-12 15:32:56 EST
smstools3/src/stats.c does:

 40 #ifndef NOSTATS
 41   MM_create(DEVICES*sizeof(_stats),tempnam(0,0));
 42 #endif

and libmm does:

235 #if defined(MM_SHMT_MMPOSX) || defined(MM_SHMT_MMFILE)
236     sprintf(shmfilename, "%s.mem", file);
237     fnmem = shmfilename;
238 #endif

257     shm_unlink(fnmem); /* Ok when it fails */
258     if ((fdmem = shm_open(fnmem, O_RDWR|O_CREAT|O_EXCL, MM_CORE_FILEMODE))
== -1)
259         FAIL(MM_ERR_CORE|MM_ERR_SYSTEM, "failed to open tempfile");

This leaves possibility that some user guesses the temporary file name generated
by adding ".mem" suffix to result of tempnam() call and cause smsd to fail. As
this can be considered a low severity security flaw, it needs to be addressed.

It would be hard to fix it, since the problem lies partly in the mm library
(which mangles the file name by adding the .mem suffix), probably by using a
private temporary directory.

I think the best solution would be to disable stats support (which is even
disabled by upstream by default and would permit dropping of mm dependency).
Comment 12 Lubomir Kundrak 2007-11-13 09:23:06 EST
All outstanding inssues have been addressed in
http://mmahut.fedorapeople.org/reviews/smstools/3.0.10/

APPROVED
Comment 13 Ville Skyttä 2007-11-13 13:44:26 EST
*** Bug 235637 has been marked as a duplicate of this bug. ***
Comment 14 Marek Mahut 2007-11-13 14:24:19 EST
Thank you Lubomir for your patience :)
Comment 15 Marek Mahut 2007-11-13 14:26:14 EST
New Package CVS Request
=======================
Package Name: smstools
Short Description: Tools to send and receive short messages through GSM modems
or mobile phones
Owners: mmahut
Branches: F-7 F-8 EL-4 EL-5
Cvsextras Commits: yes
Comment 16 Kevin Fenzi 2007-11-13 15:14:17 EST
cvs done.
Comment 17 Marek Mahut 2007-11-21 05:51:10 EST
Thank you all!
Comment 18 Fedora Update System 2007-11-21 22:35:16 EST
smstools-3.0.10-1.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update smstools'
Comment 19 Fedora Update System 2007-11-26 13:51:32 EST
smstools-3.0.10-1.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Patrick C. F. Ernzer 2014-06-29 12:11:42 EDT
Package Change Request
======================
Package Name: smstools
New Branches: epel7
Owners: mmahut pcfe
Comment 21 Jon Ciesla 2014-06-30 08:02:33 EDT
Git done (by process-git-requests).
Comment 22 James Hogarth 2015-12-03 20:06:29 EST
This was left opened following a new branch request. 

Closing again in an effort to clean up the queue.

Note You need to log in before you can comment on or make changes to this bug.