* [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c
@ 2017-11-12 7:13 Peter Müller
2017-11-14 14:37 ` Michael Tremer
0 siblings, 1 reply; 3+ messages in thread
From: Peter Müller @ 2017-11-12 7:13 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 2498 bytes --]
Make syslogctrl.c use TCP as remote logging file if specified so.
NOTE: This patch likely contains errors, since I do not know anything
about C at all. Please have a close look at it. Sorry.
Signed-off-by: Peter Müller <peter.mueller(a)link38.eu>
---
src/misc-progs/syslogdctrl.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
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 @@
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 = 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);
@@ -73,6 +74,12 @@ int main(void)
exit(ERR_SETTINGS);
}
+ if (!findkey(kv, "REMOTELOG_PROTOCOL", protocol))
+ {
+ fprintf(stderr, "Cannot read REMOTELOG_PROTOCOL\n");
+ exit(ERR_SETTINGS);
+ }
+
freekeyvalues(kv);
@@ -105,9 +112,22 @@ int main(void)
exit(ERR_CONFIG);
}
+ /* 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 == "udp" )
+ {
+ snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@\\).\\+$/\\1%s/' /etc/syslog.conf >&%d", hostname, config_fd );
+ }
+ elif ( protocol == "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 );
/* if the return code isn't 0 failsafe */
--
2.13.6
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c
2017-11-12 7:13 [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c Peter Müller
@ 2017-11-14 14:37 ` Michael Tremer
2017-11-16 21:13 ` Peter Müller
0 siblings, 1 reply; 3+ messages in thread
From: Michael Tremer @ 2017-11-14 14:37 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 3545 bytes --]
Hello,
On Sun, 2017-11-12 at 08:13 +0100, Peter Müller wrote:
> Make syslogctrl.c use TCP as remote logging file if specified so.
>
> NOTE: This patch likely contains errors, since I do not know anything
> about C at all. Please have a close look at it. Sorry.
Yes it has some errors and won't work. Not sure how you tested this
successfully.
> Signed-off-by: Peter Müller <peter.mueller(a)link38.eu>
> ---
> src/misc-progs/syslogdctrl.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> 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 @@
>
> 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 = 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);
There is no need to call memset here, but since it is for everything else it
probably doesn't hurt.
Better would be to initialise the string with "" right when it is defined. Would
spare the memset at runtime.
Or to change findkey() to set the string to "" when the key wasn't found.
> @@ -73,6 +74,12 @@ int main(void)
> exit(ERR_SETTINGS);
> }
>
> + if (!findkey(kv, "REMOTELOG_PROTOCOL", protocol))
> + {
> + fprintf(stderr, "Cannot read REMOTELOG_PROTOCOL\n");
> + exit(ERR_SETTINGS);
> + }
> +
> freekeyvalues(kv);
Should we not default to UDP here in case the value is not being set?
I think exiting is a bit much.
> @@ -105,9 +112,22 @@ int main(void)
> exit(ERR_CONFIG);
> }
>
> + /* 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 == "udp" )
> + {
> + snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@\\).\\+$/\\1%s/' /etc/syslog.conf >&%d",
> hostname, config_fd );
> + }
> + elif ( protocol == "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 );
>
> /* if the return code isn't 0 failsafe */
This block is slightly messy with calling all the seds. But I it has been like
this before, performance is not important here, etc.
But you can't compare strings in C like this. You will compare the pointer
values here which will never match.
The correct check would be: strcmp(protocol, "udp") == 0
And likewise for TCP.
Best,
-Michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c
2017-11-14 14:37 ` Michael Tremer
@ 2017-11-16 21:13 ` Peter Müller
0 siblings, 0 replies; 3+ messages in thread
From: Peter Müller @ 2017-11-16 21:13 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 3885 bytes --]
Hello Michael,
thanks for the reply. I'll have a look at this at the weekend...
Best regards,
Peter Müller
> Hello,
>
> On Sun, 2017-11-12 at 08:13 +0100, Peter Müller wrote:
> > Make syslogctrl.c use TCP as remote logging file if specified so.
> >
> > NOTE: This patch likely contains errors, since I do not know anything
> > about C at all. Please have a close look at it. Sorry.
>
> Yes it has some errors and won't work. Not sure how you tested this
> successfully.
>
> > Signed-off-by: Peter Müller <peter.mueller(a)link38.eu>
> > ---
> > src/misc-progs/syslogdctrl.c | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > 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 @@
> >
> > 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 = 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);
>
> There is no need to call memset here, but since it is for everything else it
> probably doesn't hurt.
>
> Better would be to initialise the string with "" right when it is defined. Would
> spare the memset at runtime.
>
> Or to change findkey() to set the string to "" when the key wasn't found.
>
> > @@ -73,6 +74,12 @@ int main(void)
> > exit(ERR_SETTINGS);
> > }
> >
> > + if (!findkey(kv, "REMOTELOG_PROTOCOL", protocol))
> > + {
> > + fprintf(stderr, "Cannot read REMOTELOG_PROTOCOL\n");
> > + exit(ERR_SETTINGS);
> > + }
> > +
> > freekeyvalues(kv);
>
> Should we not default to UDP here in case the value is not being set?
>
> I think exiting is a bit much.
>
> > @@ -105,9 +112,22 @@ int main(void)
> > exit(ERR_CONFIG);
> > }
> >
> > + /* 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 == "udp" )
> > + {
> > + snprintf(buffer, STRING_SIZE - 1, "/bin/sed -e
> > 's/^#\\?\\(\\*\\.\\*[[:blank:]]\\+@\\).\\+$/\\1%s/' /etc/syslog.conf >&%d",
> > hostname, config_fd );
> > + }
> > + elif ( protocol == "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 );
> >
> > /* if the return code isn't 0 failsafe */
>
> This block is slightly messy with calling all the seds. But I it has been like
> this before, performance is not important here, etc.
>
> But you can't compare strings in C like this. You will compare the pointer
> values here which will never match.
>
> The correct check would be: strcmp(protocol, "udp") == 0
>
> And likewise for TCP.
>
> Best,
> -Michael
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-16 21:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-12 7:13 [PATCH 1/3] allow remote syslog via TCP in syslogdctrl.c Peter Müller
2017-11-14 14:37 ` Michael Tremer
2017-11-16 21:13 ` Peter Müller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox