[PATCH] UTIL: Free log message when using journald - sssd-devel - Fedora mailing-lists

List overview All Threads
Download

[PATCH] UTIL: Free log message when using journald

[PATCH] Initialize sid_str to...

[PATCH] confdb: Make offline...

Jakub Hrozek

21 Oct 2013 21 Oct '13

4:01 a.m.

I was exercising sssd with valgrind when looking for #1020945 and found this leak.

Attachments:

0001-UTIL-Free-log-message-when-using-journald.patch (text/plain — 769 bytes)

Reply

Show replies by date

Pavel Březina

21 Oct 21 Oct

5:33 a.m.

On 10/21/2013 11:01 AM, Jakub Hrozek wrote:

I was exercising sssd with valgrind when looking for #1020945 and found this leak.

Ack.

Reply

Simo Sorce

9:35 a.m.

On Mon, 2013-10-21 at 11:01 +0200, Jakub Hrozek wrote:

I was exercising sssd with valgrind when looking for #1020945 and found this leak.

plain text document attachment (0001-UTIL-Free-log-message-when-using-journald.patch)

From 3bde2833eca1fc280214da03f067e3df63040899 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Sun, 20 Oct 2013 16:52:06 +0200 Subject: [PATCH 2/3] UTIL: Free log message when using journald

src/util/sss_log.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/util/sss_log.c b/src/util/sss_log.c index 5be9e7f2bc1486d83393d2d105c1a73322d7f6f1..b6b922785b03f8d9673aebdeb33a83c3f6b14076 100644 --- a/src/util/sss_log.c +++ b/src/util/sss_log.c @@ -88,6 +88,8 @@ void sss_log(int priority, const char *format, ...) "SYSLOG_FACILITY=%i", LOG_FAC(LOG_DAEMON), "SYSLOG_IDENTIFIER=%s", debug_prg_name, NULL);

free(message);

}

#else /* WITH_JOURNALD */

Is there any reason why we are not using talloc_vasrpintf() here and passing in a memcontext ?

I think I would rather fix this problem that way.

Simo.

-- Simo Sorce * Red Hat, Inc * New York

Reply

Jakub Hrozek

10:32 a.m.

On Mon, Oct 21, 2013 at 10:35:34AM -0400, Simo Sorce wrote:

On Mon, 2013-10-21 at 11:01 +0200, Jakub Hrozek wrote:

...
I was exercising sssd with valgrind when looking for #1020945 and found this leak.

plain text document attachment (0001-UTIL-Free-log-message-when-using-journald.patch)

From 3bde2833eca1fc280214da03f067e3df63040899 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Sun, 20 Oct 2013 16:52:06 +0200 Subject: [PATCH 2/3] UTIL: Free log message when using journald

src/util/sss_log.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/util/sss_log.c b/src/util/sss_log.c index 5be9e7f2bc1486d83393d2d105c1a73322d7f6f1..b6b922785b03f8d9673aebdeb33a83c3f6b14076 100644 --- a/src/util/sss_log.c +++ b/src/util/sss_log.c @@ -88,6 +88,8 @@ void sss_log(int priority, const char *format, ...) "SYSLOG_FACILITY=%i", LOG_FAC(LOG_DAEMON), "SYSLOG_IDENTIFIER=%s", debug_prg_name, NULL);

free(message);

}

#else /* WITH_JOURNALD */

Is there any reason why we are not using talloc_vasrpintf() here and passing in a memcontext ?

I think I would rather fix this problem that way.

Simo.

Mostly because I copied the old syslog-based sss_log() function.

But I don't think the mem_ctx is needed, the function doesn't allocate anything for its consumer, nor does it operate on any predictable structure (like state or tevent_req with tevent contexts), so I would prefer not to add a memory context to its signature.

Reply

Jakub Hrozek

5 Nov 5 Nov

5 a.m.

On Mon, Oct 21, 2013 at 05:32:26PM +0200, Jakub Hrozek wrote:

On Mon, Oct 21, 2013 at 10:35:34AM -0400, Simo Sorce wrote:

...
On Mon, 2013-10-21 at 11:01 +0200, Jakub Hrozek wrote:

...
I was exercising sssd with valgrind when looking for #1020945 and found this leak.

plain text document attachment (0001-UTIL-Free-log-message-when-using-journald.patch)

From 3bde2833eca1fc280214da03f067e3df63040899 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Sun, 20 Oct 2013 16:52:06 +0200 Subject: [PATCH 2/3] UTIL: Free log message when using journald

src/util/sss_log.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/util/sss_log.c b/src/util/sss_log.c index 5be9e7f2bc1486d83393d2d105c1a73322d7f6f1..b6b922785b03f8d9673aebdeb33a83c3f6b14076 100644 --- a/src/util/sss_log.c +++ b/src/util/sss_log.c @@ -88,6 +88,8 @@ void sss_log(int priority, const char *format, ...) "SYSLOG_FACILITY=%i", LOG_FAC(LOG_DAEMON), "SYSLOG_IDENTIFIER=%s", debug_prg_name, NULL);

free(message);

}

#else /* WITH_JOURNALD */

Is there any reason why we are not using talloc_vasrpintf() here and passing in a memcontext ?

I think I would rather fix this problem that way.

Simo.

Mostly because I copied the old syslog-based sss_log() function.

But I don't think the mem_ctx is needed, the function doesn't allocate anything for its consumer, nor does it operate on any predictable structure (like state or tevent_req with tevent contexts), so I would prefer not to add a memory context to its signature.

OK to push the patch as-is ?

Reply

Pavel Březina

5:15 a.m.

On 11/05/2013 12:00 PM, Jakub Hrozek wrote:

On Mon, Oct 21, 2013 at 05:32:26PM +0200, Jakub Hrozek wrote:

...
On Mon, Oct 21, 2013 at 10:35:34AM -0400, Simo Sorce wrote:

...
On Mon, 2013-10-21 at 11:01 +0200, Jakub Hrozek wrote:

...
I was exercising sssd with valgrind when looking for #1020945 and found this leak.

plain text document attachment (0001-UTIL-Free-log-message-when-using-journald.patch)

From 3bde2833eca1fc280214da03f067e3df63040899 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Sun, 20 Oct 2013 16:52:06 +0200 Subject: [PATCH 2/3] UTIL: Free log message when using journald

src/util/sss_log.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/util/sss_log.c b/src/util/sss_log.c index 5be9e7f2bc1486d83393d2d105c1a73322d7f6f1..b6b922785b03f8d9673aebdeb33a83c3f6b14076 100644 --- a/src/util/sss_log.c +++ b/src/util/sss_log.c @@ -88,6 +88,8 @@ void sss_log(int priority, const char *format, ...) "SYSLOG_FACILITY=%i", LOG_FAC(LOG_DAEMON), "SYSLOG_IDENTIFIER=%s", debug_prg_name, NULL);

free(message); }

#else /* WITH_JOURNALD */

Is there any reason why we are not using talloc_vasrpintf() here and passing in a memcontext ?

I think I would rather fix this problem that way.

Simo.

Mostly because I copied the old syslog-based sss_log() function.

But I don't think the mem_ctx is needed, the function doesn't allocate anything for its consumer, nor does it operate on any predictable structure (like state or tevent_req with tevent contexts), so I would prefer not to add a memory context to its signature.

OK to push the patch as-is ?

Yes by me.

Reply

Jakub Hrozek

7 Nov 7 Nov

4:43 a.m.

On Tue, Nov 05, 2013 at 12:15:35PM +0100, Pavel Březina wrote:

On 11/05/2013 12:00 PM, Jakub Hrozek wrote:

...
On Mon, Oct 21, 2013 at 05:32:26PM +0200, Jakub Hrozek wrote:

...
On Mon, Oct 21, 2013 at 10:35:34AM -0400, Simo Sorce wrote:

...
On Mon, 2013-10-21 at 11:01 +0200, Jakub Hrozek wrote:

...
I was exercising sssd with valgrind when looking for #1020945 and found this leak.

plain text document attachment (0001-UTIL-Free-log-message-when-using-journald.patch)

From 3bde2833eca1fc280214da03f067e3df63040899 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Sun, 20 Oct 2013 16:52:06 +0200 Subject: [PATCH 2/3] UTIL: Free log message when using journald

src/util/sss_log.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/util/sss_log.c b/src/util/sss_log.c index 5be9e7f2bc1486d83393d2d105c1a73322d7f6f1..b6b922785b03f8d9673aebdeb33a83c3f6b14076 100644 --- a/src/util/sss_log.c +++ b/src/util/sss_log.c @@ -88,6 +88,8 @@ void sss_log(int priority, const char *format, ...) "SYSLOG_FACILITY=%i", LOG_FAC(LOG_DAEMON), "SYSLOG_IDENTIFIER=%s", debug_prg_name, NULL);

free(message);

}

#else /* WITH_JOURNALD */

Is there any reason why we are not using talloc_vasrpintf() here and passing in a memcontext ?

I think I would rather fix this problem that way.

Simo.

Mostly because I copied the old syslog-based sss_log() function.

But I don't think the mem_ctx is needed, the function doesn't allocate anything for its consumer, nor does it operate on any predictable structure (like state or tevent_req with tevent contexts), so I would prefer not to add a memory context to its signature.

OK to push the patch as-is ?

Yes by me.

Pushed to master.

Reply

4412

Age (days ago)

4429

Last active (days ago)

sssd-devel@lists.fedorahosted.org

6 comments

3 participants

tags (0)

participants (3)

Jakub Hrozek
Pavel Březina
Simo Sorce