From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peter =?utf-8?q?M=C3=BCller?= <peter.mueller@link38.eu>
To: development@lists.ipfire.org
Subject: Re: [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c
Date: Thu, 16 Nov 2017 22:13:08 +0100
Message-ID: <20171116221308.706ab1f0.peter.mueller@link38.eu>
In-Reply-To: <1510670253.4838.430.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6951913146026852587=="
List-Id: <development.lists.ipfire.org>

--===============6951913146026852587==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Michael,

thanks for the reply. I'll have a look at this at the weekend...

Best regards,
Peter M=C3=BCller

> Hello,
>=20
> On Sun, 2017-11-12 at 08:13 +0100, Peter M=C3=BCller wrote:
> > Make syslogctrl.c use TCP as remote logging file if specified so.
> >=20
> > NOTE: This patch likely contains errors, since I do not know anything
> > about C at all. Please have a close look at it. Sorry. =20
>=20
> Yes it has some errors and won't work. Not sure how you tested this
> successfully.
>=20
> > Signed-off-by: Peter M=C3=BCller <peter.mueller(a)link38.eu>
> > ---
> >  src/misc-progs/syslogdctrl.c | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/src/misc-progs/syslogdctrl.c b/src/misc-progs/syslogdctrl.c
> > index 52719023e..b356ebe49 100644
> > --- a/src/misc-progs/syslogdctrl.c
> > +++ b/src/misc-progs/syslogdctrl.c
> > @@ -32,13 +32,14 @@
> > =20
> >  int main(void)
> >  {
> > -   char buffer[STRING_SIZE], command[STRING_SIZE], hostname[STRING_SIZE];
> > +   char buffer[STRING_SIZE], command[STRING_SIZE], hostname[STRING_SIZE],
> > protocol[STRING_SIZE];
> >     char varmessages[STRING_SIZE], asynclog[STRING_SIZE];
> >     int config_fd,rc,fd,pid;
> >     struct stat st;
> >     struct keyvalue *kv =3D NULL;
> >     memset(buffer, 0, STRING_SIZE);
> >     memset(hostname, 0, STRING_SIZE);
> > +   memset(protocol, 0, STRING_SIZE);
> >     memset(varmessages, 0, STRING_SIZE);
> >     memset(asynclog, 0, STRING_SIZE); =20
>=20
> There is no need to call memset here, but since it is for everything else it
> probably doesn't hurt.
>=20
> Better would be to initialise the string with "" right when it is defined. =
Would
> spare the memset at runtime.
>=20
> Or to change findkey() to set the string to "" when the key wasn't found.
>=20
> > @@ -73,6 +74,12 @@ int main(void)
> >        exit(ERR_SETTINGS);
> >     }
> > =20
> > +   if (!findkey(kv, "REMOTELOG_PROTOCOL", protocol))
> > +   {
> > +      fprintf(stderr, "Cannot read REMOTELOG_PROTOCOL\n");
> > +      exit(ERR_SETTINGS);
> > +   }
> > +
> >     freekeyvalues(kv); =20
>=20
> Should we not default to UDP here in case the value is not being set?
>=20
> I think exiting is a bit much.
>  =20
> > @@ -105,9 +112,22 @@ int main(void)
> >        exit(ERR_CONFIG);
> >     }
> > =20
> > +   /* differ between UDP and TCP as rsyslog protocol */
> >     if (!strcmp(buffer,"on"))
> > -      snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> > 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@\\).\\+$/\\1%s/' /etc/syslog.conf >&%d=
",
> > hostname, config_fd );
> > +      if ( protocol =3D=3D "udp" )
> > +      {
> > +         snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> > 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@\\).\\+$/\\1%s/' /etc/syslog.conf >&%d=
",
> > hostname, config_fd );
> > +      }
> > +      elif ( protocol =3D=3D "tcp" )
> > +      {
> > +         snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> > 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@@\\).\\+$/\\1%s/' /etc/syslog.conf >&%=
d",
> > hostname, config_fd );
> > +      }
> > +      else
> > +      {
> > +         fprintf(stderr, "Received invalid protocol for remote log\n");
> > +      }
> >     else
> > +      /* turn off remote syslog if specified */
> >        snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> > 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+(a).\\+\\)$/#\\1/' /etc/syslog.conf >&%=
d",
> > config_fd );
> > =20
> >       /* if the return code isn't 0 failsafe */ =20
>=20
> This block is slightly messy with calling all the seds. But I it has been l=
ike
> this before, performance is not important here, etc.
>=20
> But you can't compare strings in C like this. You will compare the pointer
> values here which will never match.
>=20
> The correct check would be: strcmp(protocol, "udp") =3D=3D 0
>=20
> And likewise for TCP.
>=20
> Best,
> -Michael


--===============6951913146026852587==--