Mailing List Archive

I reviewed your patch, and think it looks reasonable.

If you address the following issues and send a new patch, I'll test
(very lightly) on my system and commit it:

avoid stray changes to whitespace, eg in lib/vty.c:
-
+

diff against the head of cvs, e.g. the following conflicts are
present:
-AM_INIT_AUTOMAKE()
+AM_INIT_AUTOMAKE(quagga, 0.96.4)

blank line between decls and code to match existing style, eg.
+ mode_t oldumask;
+ oldumask = umask (0777 & ~LOGFILE_MASK);

include ChangeLog and NEWS entries (in message rather than
patch is fine/preferred)

--
Greg Troxel <gdt@ir.bbn.com>

On Tue, Dec 16, 2003 at 09:16:15AM -0500, Greg Troxel wrote:
> If you address the following issues and send a new patch, I'll test
> (very lightly) on my system and commit it:
*me happy*

> diff against the head of cvs, e.g. the following conflicts are
> present:
> -AM_INIT_AUTOMAKE()
> +AM_INIT_AUTOMAKE(quagga, 0.96.4)
My system (Debian unstable) refused to run automake with the
original line:

-=(~/debian/quagga/q2)$ make
cd . && /bin/sh /home/ch/debian/quagga/q2/missing --run aclocal
cd . && \
/bin/sh /home/ch/debian/quagga/q2/missing --run automake --gnu Makefile
automake: configure.ac: `AM_INIT_AUTOMAKE' must be used
Can't call method "close" on an undefined value at /usr/bin/automake line 7751.
make: *** [Makefile.in] Fehler 1

The info page says:
`AM_INIT_AUTOMAKE'
Runs many macros that most `configure.in''s need. This macro has
two required arguments, the package and the version number. By
default this macro `AC_DEFINE''s `PACKAGE' and `VERSION'. This
can be avoided by passing in a non-empty third argument.

> blank line between decls and code to match existing style, eg.
> + mode_t oldumask;
> + oldumask = umask (0777 & ~LOGFILE_MASK);
ok

> include ChangeLog and NEWS entries (in message rather than
> patch is fine/preferred)
ok

bye,

-christian-

--
"Arp! Arp!" - the mating call of the lonely packet

Hello

On Tue, Dec 16, 2003 at 09:16:15AM -0500, Greg Troxel wrote:
> I reviewed your patch, and think it looks reasonable.
...
> include ChangeLog and NEWS entries (in message rather than
> patch is fine/preferred)

Ok, cleaned version attached.

---------------------------snipp-- NEWS ----------
* Changes in Quagga 0.96.5

- File permissions now configurable at build time.
--------------------------snipp-- ChangeLog -----------
2003-12-21 Christian Hammers <ch@lathspell.de>

* configure.ac (and everywhere a regular file is opened for
writing): the file permissions can now be defined with
configure options.
-------------------------------------------------------

bye,

-christian-

On Sun, Dec 21, 2003 at 08:39:51PM +0100, Christian Hammers wrote:
> Ok, cleaned version attached.

Just ignore the fist block of vtysh.c, it does obviouly not belong in
this patch :-( Sorry!

bye,

-christian-

> diff --recursive --ignore-space-change -u --exclude configure --exclude '*Makefile.in' --exclude debian --exclude '*~' --exclude ltmain.sh --exclude 'config.*' --exclude aclocal.m4 quagga-0.96.4/vtysh/vtysh.c q2/vtysh/vtysh.c
> --- quagga-0.96.4/vtysh/vtysh.c 2003-06-25 12:49:55.000000000 +0200
> +++ q2/vtysh/vtysh.c 2003-12-16 00:40:39.000000000 +0100
> @@ -351,8 +351,14 @@
> vtysh_pager_name = getenv ("VTYSH_PAGER");
> if (! vtysh_pager_name)
> vtysh_pager_name = getenv ("PAGER");
> - if (! vtysh_pager_name)
> + if (! vtysh_pager_name) {
> + struct stat pager_stat;
> + if (stat("/usr/bin/pager", &pager_stat) == 0) {
> + vtysh_pager_name = "/usr/bin/pager";
> + } else {
> vtysh_pager_name = "more";
> + }
> + }
> }
>
> /* Command execution over the vty interface. */