Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-17 Thread dexter 
dexter has abandoned this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Abandoned

After all I think this patch was not the best Idea. Looking through the 
projects and correcting the loglevels if needed is better I think. Also I see 
that the print to stderr messes up some unit-tests.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: abandon

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread laforge 
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Patch Set 4: Code-Review-1

I would think it's sufficient to go through all applications (it's not that 
many, after all) and ensure that the current code doesn't have any compiled-in 
default log level below NOTICE or INFO.  In the future we should pay attention 
during code review to prevent new such bugs from being introduced.  I would 
expect they date back to our pre-gerrit development days.

This way thre's no need for enforcement in libosmocore, which may create 
problems as pau indicates.


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 15 Jan 2020 16:04:36 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Patch Set 4: Code-Review-1

I think it makes more sense to have that as unit tests rather than every time 
an app is run...


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 15 Jan 2020 14:57:12 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread dexter 
dexter has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Patch Set 4:

(2 comments)

https://gerrit.osmocom.org/c/libosmocore/+/16833/1/src/application.c
File src/application.c:

https://gerrit.osmocom.org/c/libosmocore/+/16833/1/src/application.c@122
PS1, Line 122: static void check_loglevels(const struct log_info *log_info)
> I don't really like forcing this. […]
Hmm. Maybe its a bad idea then to enforce default loglevels at all?

See ticket: http://osmocom.org/issues/2577


https://gerrit.osmocom.org/c/libosmocore/+/16833/2/src/application.c
File src/application.c:

https://gerrit.osmocom.org/c/libosmocore/+/16833/2/src/application.c@127
PS2, Line 127: printf
> We shall not log to stdout, because some programs may use it for writing 
> binary data (e.g. […]
Done



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 15 Jan 2020 12:11:28 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread dexter 
Hello fixeria, pespin, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/libosmocore/+/16833

to look at the new patch set (#4).

Change subject: application.c: check default loglevels on startup
..

application.c: check default loglevels on startup

Default log levels should not be set lower than LOGL_NOTICE. If a lower
loglevel is desired, this loglevel should be set via a configuration
file. Lets print a warning if a default loglevel lower than LOGL_NOTICE
is set to remind the programmer to set proper default loglevels.

Related: OS#2577
Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
---
M src/application.c
1 file changed, 18 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/16833/4
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread fixeria 
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/c/libosmocore/+/16833/2/src/application.c
File src/application.c:

https://gerrit.osmocom.org/c/libosmocore/+/16833/2/src/application.c@127
PS2, Line 127: printf
We shall not log to stdout, because some programs may use it for writing binary 
data (e.g. osmo-gapk). Better use stderr.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 15 Jan 2020 11:27:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-15 Thread dexter 
Hello pespin, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/libosmocore/+/16833

to look at the new patch set (#2).

Change subject: application.c: check default loglevels on startup
..

application.c: check default loglevels on startup

Default log levels should not be set lower than LOGL_NOTICE. If a lower
loglevel is desired, this loglevel should be set via a configuration
file. Lets print a warning if a default loglevel lower than LOGL_NOTICE
is set to remind the programmer to set proper default loglevels.

Related: OS#2577
Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
---
M src/application.c
1 file changed, 17 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/16833/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-13 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )

Change subject: application.c: check default loglevels on startup
..


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/c/libosmocore/+/16833/1/src/application.c
File src/application.c:

https://gerrit.osmocom.org/c/libosmocore/+/16833/1/src/application.c@122
PS1, Line 122: static void check_loglevels(const struct log_info *log_info)
I don't really like forcing this. I may want to create an app using libosmocore 
with default INFO levels.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 1
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 13 Jan 2020 14:43:17 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in libosmocore[master]: application.c: check default loglevels on startup

2020-01-13 Thread dexter 
dexter has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/16833 )


Change subject: application.c: check default loglevels on startup
..

application.c: check default loglevels on startup

Default log levels should not be set lower than LOGL_NOTICE. If a lower
loglevel is desired, this loglevel should be set via a configuration
file. Lets print a warning if a default loglevel lower than LOGL_NOTICE
is set to remind the programmer to set proper default loglevels.

Related: OS#2577
Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
---
M src/application.c
1 file changed, 17 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/16833/1

diff --git a/src/application.c b/src/application.c
index 7fd6280..a7ed01e 100644
--- a/src/application.c
+++ b/src/application.c
@@ -117,6 +117,21 @@
return osmo_init_logging2(NULL, log_info);
 }

+/* Check preset loglevels. This check prints a warning if the default loglevel
+ * is lower than LOGL_NOTICE, which should never be the default */
+static void check_loglevels(const struct log_info *log_info)
+{
+   unsigned int i;
+   for (i = 0; i < log_info->num_cat; i++) {
+   if (log_info->cat[i].loglevel < LOGL_NOTICE) {
+   printf
+   ("FIXME: The default loglevel of %s is set to %s. 
Default log levels should not be lower than LOGL_NOTICE -- please fix!\n",
+log_info->cat[i].name,
+log_level_name(log_info->cat[i].loglevel));
+   }
+   }
+}
+
 int osmo_init_logging2(void *ctx, const struct log_info *log_info)
 {
static int logging_initialized = 0;
@@ -132,6 +147,8 @@

log_add_target(osmo_stderr_target);
log_set_all_filter(osmo_stderr_target, 1);
+   check_loglevels(log_info);
+
return 0;
 }


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16833
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d7e345c14a188430a0e991bfd9fb0343d05ea92
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 1
Gerrit-Owner: dexter 
Gerrit-MessageType: newchange