diff mbox

libosmocore minor issues

Message ID 1076717592.8478548.1447255391492.JavaMail.zimbra@redhat.com
State Accepted
Headers show

Commit Message

Jaroslav Skarvada Nov. 11, 2015, 3:23 p.m. UTC
----- Original Message -----
> Hi Jaroslav,
> 
> On Tue, Nov 10, 2015 at 04:56:23AM -0500, Jaroslav Skarvada wrote:
> > I am working on getting libosmocom package into Fedora, there is
> > review request in [1].
> 
> Thanks for this!
>  
> > During the packaging I found following minor issues:
> > - Incorrect FSF addresses in source code
> 
> This is easy to fix.  Do you already have a patch to fix all
> occurrences?  I'd happily apply that.
> 
Patch attempting to fix it is attached.

> > - exit call in the library: /usr/lib64/libosmovty.so.3.0.0 exit@GLIBC_2.2.5
> > libraries shouldn't generally call exit.
> 
> libosmovty started as a fork of the VTY (telnet command line interface)
> code of GNU zebra.  So the code was not written as a library to begin
> with, but was part of zebra itself.
> 
> In any case, addressing those issues is not particularly easy, as said
> functions should never fail at this point, but they don't have a way to
> return an error code, or their callers simply assume they always
> succeed.
> 
> So the best we can do without breaking API and ABI (and modifying all
> users of the library) is to change the exit(1) calls into an assert.
> Would that be better?

Nice, thanks. Please note, this is not something that blocks Fedora
review, I just wanted to point it out, e.g. to consider it for
next API update sometimes in the future

thanks & regards

Jaroslav

Comments

Harald Welte Nov. 12, 2015, 12:50 p.m. UTC | #1
Hi Jaroslav,

On Wed, Nov 11, 2015 at 10:23:11AM -0500, Jaroslav Skarvada wrote:
> > > During the packaging I found following minor issues:
> > > - Incorrect FSF addresses in source code
> > 
> Patch attempting to fix it is attached.

Patch applied, thanks.

> > > - exit call in the library: /usr/lib64/libosmovty.so.3.0.0 exit@GLIBC_2.2.5
> > > libraries shouldn't generally call exit.
> > 
> > So the best we can do without breaking API and ABI (and modifying all
> > users of the library) is to change the exit(1) calls into an assert.
> > Would that be better?
> 
> Nice, thanks. Please note, this is not something that blocks Fedora
> review, I just wanted to point it out, e.g. to consider it for
> next API update sometimes in the future

Thanks.  I've just applied a patch to change the exit() calls into
OSMO_ASSERT() instead, which is a better and more generic way to do this
anyway.

The VTY code doesn't get much love from us, we simply adapted it from
Zebra back in the early days of OpenBSC, and it sits there without much
review/improvement/change.

Regards,
	Harald
Suraev Nov. 12, 2015, 2:05 p.m. UTC | #2
12.11.2015 13:50, Harald Welte пишет:
> 
> The VTY code doesn't get much love from us, we simply adapted it from
> Zebra back in the early days of OpenBSC, and it sits there without much
> review/improvement/change.
> 

Speaking of which - upstream (quagga, successor to zebra) would welcome if anyone
could abstract their vty code and split it into separate library.

See https://bugzilla.quagga.net/show_bug.cgi?id=847 for details.

cheers,
Max.
diff mbox

Patch

From db50baf82628b88d56210945c1a109ea61715b7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaroslav=20=C5=A0karvada?= <jskarvad@redhat.com>
Date: Wed, 11 Nov 2015 16:02:54 +0100
Subject: [PATCH] fix FSF address in sources/headers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jaroslav Škarvada <jskarvad@redhat.com>
---
 include/osmocom/core/linuxrbtree.h | 3 ++-
 include/osmocom/vty/buffer.h       | 4 ++--
 include/osmocom/vty/command.h      | 4 ++--
 include/osmocom/vty/vector.h       | 4 ++--
 src/rbtree.c                       | 3 ++-
 src/select.c                       | 3 ++-
 src/vty/buffer.c                   | 4 ++--
 src/vty/command.c                  | 4 ++--
 src/vty/vector.c                   | 4 ++--
 9 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/include/osmocom/core/linuxrbtree.h b/include/osmocom/core/linuxrbtree.h
index ef8bc15..d3f9fd1 100644
--- a/include/osmocom/core/linuxrbtree.h
+++ b/include/osmocom/core/linuxrbtree.h
@@ -14,7 +14,8 @@ 
 
   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
-  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+  MA  02110-1301, USA.
 
   linux/include/linux/rbtree.h
 
diff --git a/include/osmocom/vty/buffer.h b/include/osmocom/vty/buffer.h
index f6c86a1..56c28f0 100644
--- a/include/osmocom/vty/buffer.h
+++ b/include/osmocom/vty/buffer.h
@@ -16,8 +16,8 @@ 
  *
  * You should have received a copy of the GNU General Public License
  * along with GNU Zebra; see the file COPYING.  If not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 02111-1307, USA.
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301, USA.
  */
 
 #pragma once
diff --git a/include/osmocom/vty/command.h b/include/osmocom/vty/command.h
index 2ef4109..2078e1b 100644
--- a/include/osmocom/vty/command.h
+++ b/include/osmocom/vty/command.h
@@ -16,8 +16,8 @@ 
  *
  * You should have received a copy of the GNU General Public License
  * along with GNU Zebra; see the file COPYING.  If not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 02111-1307, USA.
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301, USA.
  */
 
 #pragma once
diff --git a/include/osmocom/vty/vector.h b/include/osmocom/vty/vector.h
index 7caa5ff..c00804d 100644
--- a/include/osmocom/vty/vector.h
+++ b/include/osmocom/vty/vector.h
@@ -16,8 +16,8 @@ 
  *
  * You should have received a copy of the GNU General Public License
  * along with GNU Zebra; see the file COPYING.  If not, write to the Free
- * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
- * 02111-1307, USA.
+ * Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301, USA.
  */
 
 #pragma once
diff --git a/src/rbtree.c b/src/rbtree.c
index 4e7c0f3..f0ebb8c 100644
--- a/src/rbtree.c
+++ b/src/rbtree.c
@@ -15,7 +15,8 @@ 
 
   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
-  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+  MA  02110-1301, USA
 
   linux/lib/rbtree.c
 */
diff --git a/src/select.c b/src/select.c
index b0e8b0c..5421c77 100644
--- a/src/select.c
+++ b/src/select.c
@@ -16,7 +16,8 @@ 
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ *  MA  02110-1301, USA.
  */
 
 #include <fcntl.h>
diff --git a/src/vty/buffer.c b/src/vty/buffer.c
index e0abe81..8862da9 100644
--- a/src/vty/buffer.c
+++ b/src/vty/buffer.c
@@ -16,8 +16,8 @@ 
  *
  * You should have received a copy of the GNU General Public License
  * along with GNU Zebra; see the file COPYING.  If not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 02111-1307, USA.
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301, USA.
  */
 
 #include <stdio.h>
diff --git a/src/vty/command.c b/src/vty/command.c
index 290b12d..149eca3 100644
--- a/src/vty/command.c
+++ b/src/vty/command.c
@@ -18,8 +18,8 @@  General Public License for more details.
 
 You should have received a copy of the GNU General Public License
 along with GNU Zebra; see the file COPYING.  If not, write to the
-Free Software Foundation, Inc., 59 Temple Place - Suite 330,
-Boston, MA 02111-1307, USA.  */
+Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+Boston, MA  02110-1301, USA. */
 
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/src/vty/vector.c b/src/vty/vector.c
index aaff87d..c5a99af 100644
--- a/src/vty/vector.c
+++ b/src/vty/vector.c
@@ -15,8 +15,8 @@ 
  *
  * You should have received a copy of the GNU General Public License
  * along with GNU Zebra; see the file COPYING.  If not, write to the Free
- * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
- * 02111-1307, USA.
+ * Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA  02110-1301, USA.
  */
 
 #include <stdlib.h>
-- 
2.4.3