Re: [pulseaudio-discuss] [PATCH 1/2] Log feature: Add a new log target to a file descriptor

2011-03-23 Thread Maarten Bosmans
2011/3/21 Becker, VincentX :
>>>'Twas brillig, and Vincent Becker at 18/03/11 10:23 did gyre and
>>gimble:
 This patch enables logging of text debug messages (pa_log feature)
>>>into a file or a device driver.
 Example : pulseaudio --log-target=file:./mylog.txt
>>>
>>>Many thanks for your perseverence here. We were beginning to worry we
>>>were annoying you too much with constant tweaks for a relatively simple
>>>patch.
>>>
>>
>>Hi Col,
>>I actually crashed my Linux environment, this is why it took me some
>>time to recover and get back to you.
>>
>>>I did actually make a couple extra tweaks on top of this one which I've
>>>attached FYI. One of them was to fix the double close that Arun pointed
>>>out before and was still in this version.
>>>
>>>If I've cocked it up, please feel free to publicly humiliate me in a
>>>manner of your choosing.
>>>
>>
>>No worries! It's cleaner like that I admit.
>
> OOOps! I just noticed a naughty integration error of mine. Unluckily, in the 
> process of redelivering, I did not retest so in daemon-conf.c, it is 
> implemented in the wrong function pa_daemon_conf_set_log_level. It should be 
> in the function just above pa_daemon_conf_set_log_target. Both functions look 
> very similar and are one above each other, so there was still a tiny chance 
> to mess up ! Sorry, sorry..
> Should I resent by the way a corrected version ?

The first patch is already in git master, so you better send a patch
against that.

> Vincent

Maarten
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] [PATCH 1/2] Log feature: Add a new log target to a file descriptor

2011-03-23 Thread Becker, VincentX
>>'Twas brillig, and Vincent Becker at 18/03/11 10:23 did gyre and
>gimble:
>>> This patch enables logging of text debug messages (pa_log feature)
>>into a file or a device driver.
>>> Example : pulseaudio --log-target=file:./mylog.txt
>>
>>Many thanks for your perseverence here. We were beginning to worry we
>>were annoying you too much with constant tweaks for a relatively simple
>>patch.
>>
>
>Hi Col,
>I actually crashed my Linux environment, this is why it took me some
>time to recover and get back to you.
>
>>I did actually make a couple extra tweaks on top of this one which I've
>>attached FYI. One of them was to fix the double close that Arun pointed
>>out before and was still in this version.
>>
>>If I've cocked it up, please feel free to publicly humiliate me in a
>>manner of your choosing.
>>
>
>No worries! It's cleaner like that I admit.

OOOps! I just noticed a naughty integration error of mine. Unluckily, in the 
process of redelivering, I did not retest so in daemon-conf.c, it is 
implemented in the wrong function pa_daemon_conf_set_log_level. It should be in 
the function just above pa_daemon_conf_set_log_target. Both functions look very 
similar and are one above each other, so there was still a tiny chance to mess 
up ! Sorry, sorry..
Should I resent by the way a corrected version ?

Vincent

>Thanks
>Vincent
>
>>Many thanks.
>>
>>Col
>>
>>--
>>
>>Colin Guthrie
>>gmane(at)colin.guthr.ie
>>http://colin.guthr.ie/
>>
>>Day Job:
>>  Tribalogic Limited [http://www.tribalogic.net/] Open Source:
>>  Mageia Contributor [http://www.mageia.org/]
>>  PulseAudio Hacker [http://www.pulseaudio.org/]
>>  Trac Hacker [http://trac.edgewall.org/]
>-
>Intel Corporation SAS (French simplified joint stock company)
>Registered headquarters: "Les Montalets"- 2, rue de Paris,
>92196 Meudon Cedex, France
>Registration Number:  302 456 199 R.C.S. NANTERRE
>Capital: 4,572,000 Euros
>
>This e-mail and any attachments may contain confidential material for
>the sole use of the intended recipient(s). Any review or distribution
>by others is strictly prohibited. If you are not the intended
>recipient, please contact the sender and delete all copies.
>___
>pulseaudio-discuss mailing list
>pulseaudio-discuss@mail.0pointer.de
>https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] [PATCH 1/2] Log feature: Add a new log target to a file descriptor

2011-03-18 Thread Becker, VincentX
>-Original Message-
>From: pulseaudio-discuss-boun...@mail.0pointer.de [mailto:pulseaudio-
>discuss-boun...@mail.0pointer.de] On Behalf Of Colin Guthrie
>Sent: Friday, March 18, 2011 1:48 PM
>To: pulseaudio-discuss@mail.0pointer.de
>Subject: Re: [pulseaudio-discuss] [PATCH 1/2] Log feature: Add a new log
>target to a file descriptor
>
>'Twas brillig, and Vincent Becker at 18/03/11 10:23 did gyre and gimble:
>> This patch enables logging of text debug messages (pa_log feature)
>into a file or a device driver.
>> Example : pulseaudio --log-target=file:./mylog.txt
>
>Many thanks for your perseverence here. We were beginning to worry we
>were annoying you too much with constant tweaks for a relatively simple
>patch.
>

Hi Col,
I actually crashed my Linux environment, this is why it took me some time to 
recover and get back to you.

>I did actually make a couple extra tweaks on top of this one which I've
>attached FYI. One of them was to fix the double close that Arun pointed
>out before and was still in this version.
>
>If I've cocked it up, please feel free to publicly humiliate me in a
>manner of your choosing.
>

No worries! It's cleaner like that I admit.
Thanks
Vincent

>Many thanks.
>
>Col
>
>--
>
>Colin Guthrie
>gmane(at)colin.guthr.ie
>http://colin.guthr.ie/
>
>Day Job:
>  Tribalogic Limited [http://www.tribalogic.net/] Open Source:
>  Mageia Contributor [http://www.mageia.org/]
>  PulseAudio Hacker [http://www.pulseaudio.org/]
>  Trac Hacker [http://trac.edgewall.org/]
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] [PATCH 1/2] Log feature: Add a new log target to a file descriptor

2011-03-18 Thread Colin Guthrie
'Twas brillig, and Vincent Becker at 18/03/11 10:23 did gyre and gimble:
> This patch enables logging of text debug messages (pa_log feature) into a 
> file or a device driver.
> Example : pulseaudio --log-target=file:./mylog.txt

Many thanks for your perseverence here. We were beginning to worry we
were annoying you too much with constant tweaks for a relatively simple
patch.

I did actually make a couple extra tweaks on top of this one which I've
attached FYI. One of them was to fix the double close that Arun pointed
out before and was still in this version.

If I've cocked it up, please feel free to publicly humiliate me in a
manner of your choosing.

Many thanks.

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]
diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
index caa5b2b..4854aff 100644
--- a/src/daemon/cmdline.c
+++ b/src/daemon/cmdline.c
@@ -145,8 +145,8 @@ void pa_cmdline_help(const char *argv0) {
"this time passed\n"
"  --log-level[=LEVEL]   Increase or set verbosity level\n"
"  -vIncrease the verbosity level\n"
-   "  --log-target={auto,syslog,stderr,\n"
-   "file:PATH}  Specify the log target\n"
+   "  --log-target={auto,syslog,stderr,file:PATH}\n"
+   "Specify the log target\n"
"  --log-meta[=BOOL] Include code location in log messages\n"
"  --log-time[=BOOL] Include timestamps in log messages\n"
"  --log-backtrace=FRAMESInclude a backtrace in log messages\n"
diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
index 0d42f73..ce93dbc 100644
--- a/src/daemon/daemon-conf.c
+++ b/src/daemon/daemon-conf.c
@@ -142,8 +142,6 @@ static const pa_daemon_conf default_conf = {
 #endif
 };
 
-static int log_fd = -1;
-
 pa_daemon_conf* pa_daemon_conf_new(void) {
 pa_daemon_conf *c;
 
@@ -170,8 +168,7 @@ pa_daemon_conf* pa_daemon_conf_new(void) {
 void pa_daemon_conf_free(pa_daemon_conf *c) {
 pa_assert(c);
 
-if (log_fd >= 0)
-pa_close(log_fd);
+pa_log_set_fd(-1);
 
 pa_xfree(c->script_commands);
 pa_xfree(c->dl_search_path);
@@ -220,6 +217,7 @@ int pa_daemon_conf_set_log_level(pa_daemon_conf *c, const char *string) {
 c->log_level = PA_LOG_ERROR;
 else if (pa_startswith(string, "file:")) {
 char file_path[512];
+int log_fd;
 
 pa_strlcpy(file_path, string + 5, sizeof(file_path));
 
diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
index 1494906..b12cbf0 100644
--- a/src/pulsecore/log.c
+++ b/src/pulsecore/log.c
@@ -130,9 +130,12 @@ void pa_log_set_flags(pa_log_flags_t _flags, pa_log_merge_t merge) {
 }
 
 void pa_log_set_fd(int fd) {
-pa_assert(fd >= 0);
-
-log_fd = fd;
+if (fd >= 0)
+log_fd = fd;
+else if (log_fd >= 0) {
+pa_close(log_fd);
+log_fd = -1;
+}
 }
 
 void pa_log_set_show_backtrace(unsigned nlevels) {
@@ -407,18 +410,17 @@ void pa_log_levelv_meta(
 #endif
 
 case PA_LOG_FD: {
-if (log_fd != -1) {
+if (log_fd >= 0) {
 char metadata[256];
 
 pa_snprintf(metadata, sizeof(metadata), "\n%c %s %s", level_to_char[level], timestamp, location);
 
 if ((write(log_fd, metadata, strlen(metadata)) < 0) || (write(log_fd, t, strlen(t)) < 0)) {
 saved_errno = errno;
-pa_close(log_fd);
+pa_log_set_fd(-1);
 fprintf(stderr, "%s\n", "Error writing logs to a file descriptor. Redirect log messages to console.");
 fprintf(stderr, "%s %s\n", metadata, t);
 pa_log_set_target(PA_LOG_STDERR);
-log_fd = -1;
 }
 }
 
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss