Fix data sent to callback of hook_process, improve hook_process by using buffer

Fix: some data was sometimes missing (not sent to callback).
Improvement: use a 64KB buffer for child output and send data to callback only
when buffer is full.
v2.8-utf8proc
Sebastien Helleu 2010-11-14 16:22:31 +01:00
parent 2f45cbfb02
commit 4d1c9a8315
6 changed files with 203 additions and 82 deletions

View File

@ -1,12 +1,15 @@
WeeChat ChangeLog
=================
Sébastien Helleu <flashcode@flashtux.org>
v0.3.4-dev, 2010-11-12
v0.3.4-dev, 2010-11-14
Version 0.3.4 (under dev!)
--------------------------
* core: fix data sent to callback of hook_process (some data was sometimes
missing), use a 64KB buffer for child output and send data to callback only
when buffer is full
* core: fix crash when displaying groups in buffer nicklist
* core: fix bug with message "day changed to", sometimes displayed several
times wrongly

View File

@ -6165,6 +6165,17 @@ Return value:
* pointer to new hook, NULL if error occured
[NOTE]
Buffer size for sending data to callback is 64KB (there are 2 buffers: one for
stdout and one for stderr).
If output from child process (stdout or stderr) is longer than 64KB, callback
will be called more than one time.
[IMPORTANT]
Even if most of times your callback is called only once, you must ensure that
many calls to callback are ok in your code: you must concatenate data issued by
many calls and use data only when return code is nonnegative.
C example:
[source,C]

View File

@ -6242,6 +6242,18 @@ Valeur de retour :
* pointeur vers le nouveau "hook", NULL en cas d'erreur
[NOTE]
La taille du tampon pour l'envoi des données au "callback" est de 64 Ko (il y a
2 tampons: un pour stdout et un pour stderr).
Si la sortie du processus fils (stdout ou stderr) est plus longue que 64 Ko, le
"callback" sera appelé plusieurs fois.
[IMPORTANT]
Même si la plupart du temps le "callback" n'est appelé qu'une seule fois, vous
devez vous assurer que plusieurs appels au "callback" sont ok dans votre code :
vous devez concaténer les données issues de plusieurs appels et n'utiliser les
données que lorsque le code retour est positif ou nul.
Exemple en C :
[source,C]

View File

@ -6217,6 +6217,19 @@ Valore restituito:
* puntatore al nuovo hook, NULL in caso di errore
// TRANSLATION MISSING
[NOTE]
Buffer size for sending data to callback is 64KB (there are 2 buffers: one for
stdout and one for stderr).
If output from child process (stdout or stderr) is longer than 64KB, callback
will be called more than one time.
// TRANSLATION MISSING
[IMPORTANT]
Even if most of times your callback is called only once, you must ensure that
many calls to callback are ok in your code: you must concatenate data issued by
many calls and use data only when return code is nonnegative.
Esempio in C:
[source,C]

View File

@ -1273,16 +1273,34 @@ hook_process (struct t_weechat_plugin *plugin,
{
struct t_hook *new_hook;
struct t_hook_process *new_hook_process;
char *stdout_buffer, *stderr_buffer;
if (!command || !command[0] || !callback)
return NULL;
stdout_buffer = malloc (HOOK_PROCESS_BUFFER_SIZE + 1);
if (!stdout_buffer)
return NULL;
stderr_buffer = malloc (HOOK_PROCESS_BUFFER_SIZE + 1);
if (!stderr_buffer)
{
free (stdout_buffer);
return NULL;
}
new_hook = malloc (sizeof (*new_hook));
if (!new_hook)
{
free (stdout_buffer);
free (stderr_buffer);
return NULL;
}
new_hook_process = malloc (sizeof (*new_hook_process));
if (!new_hook_process)
{
free (stdout_buffer);
free (stderr_buffer);
free (new_hook);
return NULL;
}
@ -1294,14 +1312,18 @@ hook_process (struct t_weechat_plugin *plugin,
new_hook_process->callback = callback;
new_hook_process->command = strdup (command);
new_hook_process->timeout = timeout;
new_hook_process->child_stdout_read = -1;
new_hook_process->child_stdout_write = -1;
new_hook_process->child_stderr_read = -1;
new_hook_process->child_stderr_write = -1;
new_hook_process->child_read[HOOK_PROCESS_STDOUT] = -1;
new_hook_process->child_read[HOOK_PROCESS_STDERR] = -1;
new_hook_process->child_write[HOOK_PROCESS_STDOUT] = -1;
new_hook_process->child_write[HOOK_PROCESS_STDERR] = -1;
new_hook_process->child_pid = 0;
new_hook_process->hook_fd_stdout = NULL;
new_hook_process->hook_fd_stderr = NULL;
new_hook_process->hook_fd[HOOK_PROCESS_STDOUT] = NULL;
new_hook_process->hook_fd[HOOK_PROCESS_STDERR] = NULL;
new_hook_process->hook_timer = NULL;
new_hook_process->buffer[HOOK_PROCESS_STDOUT] = stdout_buffer;
new_hook_process->buffer[HOOK_PROCESS_STDERR] = stderr_buffer;
new_hook_process->buffer_size[HOOK_PROCESS_STDOUT] = 0;
new_hook_process->buffer_size[HOOK_PROCESS_STDERR] = 0;
hook_add_to_list (new_hook);
@ -1327,14 +1349,14 @@ hook_process_child (struct t_hook *hook_process)
close (STDIN_FILENO);
/* redirect stdout/stderr to pipe (so that father process can read them) */
close (HOOK_PROCESS(hook_process, child_stdout_read));
close (HOOK_PROCESS(hook_process, child_stderr_read));
if (dup2 (HOOK_PROCESS(hook_process, child_stdout_write),
close (HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDOUT]));
close (HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDERR]));
if (dup2 (HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDOUT]),
STDOUT_FILENO) < 0)
{
_exit (EXIT_FAILURE);
}
if (dup2 (HOOK_PROCESS(hook_process, child_stderr_write),
if (dup2 (HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDERR]),
STDERR_FILENO) < 0)
{
_exit (EXIT_FAILURE);
@ -1350,6 +1372,55 @@ hook_process_child (struct t_hook *hook_process)
_exit (EXIT_FAILURE);
}
/*
* hook_process_send_buffers: send buffers (stdout/stderr) to callback
*/
void
hook_process_send_buffers (struct t_hook *hook_process, int callback_rc)
{
int i, size;
/* add '\0' at end of stdout and stderr */
for (i = 0; i < 2; i++)
{
size = HOOK_PROCESS(hook_process, buffer_size[i]);
if (size > 0)
HOOK_PROCESS(hook_process, buffer[i])[size] = '\0';
}
/* send buffers to callback */
(void) (HOOK_PROCESS(hook_process, callback))
(hook_process->callback_data,
HOOK_PROCESS(hook_process, command),
callback_rc,
(HOOK_PROCESS(hook_process, buffer_size[HOOK_PROCESS_STDOUT]) > 0) ?
HOOK_PROCESS(hook_process, buffer[HOOK_PROCESS_STDOUT]) : NULL,
(HOOK_PROCESS(hook_process, buffer_size[HOOK_PROCESS_STDERR]) > 0) ?
HOOK_PROCESS(hook_process, buffer[HOOK_PROCESS_STDERR]) : NULL);
/* reset size for stdout and stderr */
HOOK_PROCESS(hook_process, buffer_size[HOOK_PROCESS_STDOUT]) = 0;
HOOK_PROCESS(hook_process, buffer_size[HOOK_PROCESS_STDERR]) = 0;
}
/*
* hook_process_add_to_buffer: add some data to buffer (stdout or stderr)
*/
void
hook_process_add_to_buffer (struct t_hook *hook_process, int index_buffer,
const char *buffer, int size)
{
if (HOOK_PROCESS(hook_process, buffer_size[index_buffer]) + size > HOOK_PROCESS_BUFFER_SIZE)
hook_process_send_buffers (hook_process, WEECHAT_HOOK_PROCESS_RUNNING);
memcpy (HOOK_PROCESS(hook_process, buffer[index_buffer]) +
HOOK_PROCESS(hook_process, buffer_size[index_buffer]),
buffer, size);
HOOK_PROCESS(hook_process, buffer_size[index_buffer]) += size;
}
/*
* hook_process_child_read: read process output (stdout or stderr) from child
* process
@ -1357,7 +1428,7 @@ hook_process_child (struct t_hook *hook_process)
void
hook_process_child_read (struct t_hook *hook_process, int fd,
int out, struct t_hook **hook_fd)
int index_buffer, struct t_hook **hook_fd)
{
char buffer[4096];
int num_read;
@ -1368,13 +1439,8 @@ hook_process_child_read (struct t_hook *hook_process, int fd,
num_read = read (fd, buffer, sizeof (buffer) - 1);
if (num_read > 0)
{
buffer[num_read] = '\0';
(void) (HOOK_PROCESS(hook_process, callback))
(hook_process->callback_data,
HOOK_PROCESS(hook_process, command),
WEECHAT_HOOK_PROCESS_RUNNING,
(out) ? buffer : NULL,
(out) ? NULL : buffer);
hook_process_add_to_buffer (hook_process, index_buffer,
buffer, num_read);
}
else if (num_read == 0)
{
@ -1394,8 +1460,8 @@ hook_process_child_read_stdout_cb (void *arg_hook_process, int fd)
struct t_hook *hook_process;
hook_process = (struct t_hook *)arg_hook_process;
hook_process_child_read (hook_process, fd, 1,
&(HOOK_PROCESS(hook_process, hook_fd_stdout)));
hook_process_child_read (hook_process, fd, HOOK_PROCESS_STDOUT,
&(HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDOUT])));
return WEECHAT_RC_OK;
}
@ -1410,8 +1476,8 @@ hook_process_child_read_stderr_cb (void *arg_hook_process, int fd)
struct t_hook *hook_process;
hook_process = (struct t_hook *)arg_hook_process;
hook_process_child_read (hook_process, fd, 0,
&(HOOK_PROCESS(hook_process, hook_fd_stderr)));
hook_process_child_read (hook_process, fd, HOOK_PROCESS_STDERR,
&(HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDERR])));
return WEECHAT_RC_OK;
}
@ -1435,23 +1501,28 @@ hook_process_timer_cb (void *arg_hook_process, int remaining_calls)
if (remaining_calls == 0)
{
hook_process_send_buffers (hook_process, WEECHAT_HOOK_PROCESS_ERROR);
gui_chat_printf (NULL,
_("End of command '%s', timeout reached (%.1fs)"),
HOOK_PROCESS(hook_process, command),
((float)HOOK_PROCESS(hook_process, timeout)) / 1000);
kill (HOOK_PROCESS(hook_process, child_pid), SIGKILL);
usleep (1000);
}
if (waitpid (HOOK_PROCESS(hook_process, child_pid), &status, WNOHANG) > 0)
{
rc = WEXITSTATUS(status);
(void) (HOOK_PROCESS(hook_process, callback))
(hook_process->callback_data,
HOOK_PROCESS(hook_process, command),
rc, NULL, NULL);
unhook (hook_process);
}
else
{
if (!HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDOUT])
&& !HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDERR]))
{
if (waitpid (HOOK_PROCESS(hook_process, child_pid), &status, WNOHANG) > 0)
{
rc = WEXITSTATUS(status);
hook_process_send_buffers (hook_process, rc);
unhook (hook_process);
}
}
}
return WEECHAT_RC_OK;
}
@ -1492,11 +1563,11 @@ hook_process_run (struct t_hook *hook_process)
return;
}
HOOK_PROCESS(hook_process, child_stdout_read) = pipe_stdout[0];
HOOK_PROCESS(hook_process, child_stdout_write) = pipe_stdout[1];
HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDOUT]) = pipe_stdout[0];
HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDOUT]) = pipe_stdout[1];
HOOK_PROCESS(hook_process, child_stderr_read) = pipe_stderr[0];
HOOK_PROCESS(hook_process, child_stderr_write) = pipe_stderr[1];
HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDERR]) = pipe_stderr[0];
HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDERR]) = pipe_stderr[1];
switch (pid = fork ())
{
@ -1519,22 +1590,25 @@ hook_process_run (struct t_hook *hook_process)
}
/* parent process */
HOOK_PROCESS(hook_process, child_pid) = pid;
close (HOOK_PROCESS(hook_process, child_stdout_write));
HOOK_PROCESS(hook_process, child_stdout_write) = -1;
close (HOOK_PROCESS(hook_process, child_stderr_write));
HOOK_PROCESS(hook_process, child_stderr_write) = -1;
HOOK_PROCESS(hook_process, hook_fd_stdout) = hook_fd (hook_process->plugin,
HOOK_PROCESS(hook_process, child_stdout_read),
1, 0, 0,
&hook_process_child_read_stdout_cb,
hook_process);
close (HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDOUT]));
HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDOUT]) = -1;
close (HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDERR]));
HOOK_PROCESS(hook_process, child_write[HOOK_PROCESS_STDERR]) = -1;
HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDOUT]) =
hook_fd (hook_process->plugin,
HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDOUT]),
1, 0, 0,
&hook_process_child_read_stdout_cb,
hook_process);
HOOK_PROCESS(hook_process, hook_fd[HOOK_PROCESS_STDERR]) =
hook_fd (hook_process->plugin,
HOOK_PROCESS(hook_process, child_read[HOOK_PROCESS_STDERR]),
1, 0, 0,
&hook_process_child_read_stderr_cb,
hook_process);
HOOK_PROCESS(hook_process, hook_fd_stderr) = hook_fd (hook_process->plugin,
HOOK_PROCESS(hook_process, child_stderr_read),
1, 0, 0,
&hook_process_child_read_stderr_cb,
hook_process);
timeout = HOOK_PROCESS(hook_process, timeout);
interval = 100;
max_calls = 0;
@ -2523,7 +2597,7 @@ unhook (struct t_hook *hook)
/* hook already deleted? */
if (hook->deleted)
return;
if (weechat_debug_core >= 2)
{
gui_chat_printf (NULL,
@ -2588,10 +2662,10 @@ unhook (struct t_hook *hook)
case HOOK_TYPE_PROCESS:
if (HOOK_PROCESS(hook, command))
free (HOOK_PROCESS(hook, command));
if (HOOK_PROCESS(hook, hook_fd_stdout))
unhook (HOOK_PROCESS(hook, hook_fd_stdout));
if (HOOK_PROCESS(hook, hook_fd_stderr))
unhook (HOOK_PROCESS(hook, hook_fd_stderr));
if (HOOK_PROCESS(hook, hook_fd[HOOK_PROCESS_STDOUT]))
unhook (HOOK_PROCESS(hook, hook_fd[HOOK_PROCESS_STDOUT]));
if (HOOK_PROCESS(hook, hook_fd[HOOK_PROCESS_STDERR]))
unhook (HOOK_PROCESS(hook, hook_fd[HOOK_PROCESS_STDERR]));
if (HOOK_PROCESS(hook, hook_timer))
unhook (HOOK_PROCESS(hook, hook_timer));
if (HOOK_PROCESS(hook, child_pid) > 0)
@ -2599,14 +2673,18 @@ unhook (struct t_hook *hook)
kill (HOOK_PROCESS(hook, child_pid), SIGKILL);
waitpid (HOOK_PROCESS(hook, child_pid), NULL, 0);
}
if (HOOK_PROCESS(hook, child_stdout_read) != -1)
close (HOOK_PROCESS(hook, child_stdout_read));
if (HOOK_PROCESS(hook, child_stdout_write) != -1)
close (HOOK_PROCESS(hook, child_stdout_write));
if (HOOK_PROCESS(hook, child_stderr_read) != -1)
close (HOOK_PROCESS(hook, child_stderr_read));
if (HOOK_PROCESS(hook, child_stderr_write) != -1)
close (HOOK_PROCESS(hook, child_stderr_write));
if (HOOK_PROCESS(hook, child_read[HOOK_PROCESS_STDOUT]) != -1)
close (HOOK_PROCESS(hook, child_read[HOOK_PROCESS_STDOUT]));
if (HOOK_PROCESS(hook, child_write[HOOK_PROCESS_STDOUT]) != -1)
close (HOOK_PROCESS(hook, child_write[HOOK_PROCESS_STDOUT]));
if (HOOK_PROCESS(hook, child_read[HOOK_PROCESS_STDERR]) != -1)
close (HOOK_PROCESS(hook, child_read[HOOK_PROCESS_STDERR]));
if (HOOK_PROCESS(hook, child_write[HOOK_PROCESS_STDERR]) != -1)
close (HOOK_PROCESS(hook, child_write[HOOK_PROCESS_STDERR]));
if (HOOK_PROCESS(hook, buffer[HOOK_PROCESS_STDOUT]))
free (HOOK_PROCESS(hook, buffer[HOOK_PROCESS_STDOUT]));
if (HOOK_PROCESS(hook, buffer[HOOK_PROCESS_STDERR]))
free (HOOK_PROCESS(hook, buffer[HOOK_PROCESS_STDERR]));
break;
case HOOK_TYPE_CONNECT:
if (HOOK_CONNECT(hook, proxy))
@ -2874,19 +2952,19 @@ hook_add_to_infolist_type (struct t_infolist *infolist,
return 0;
if (!infolist_new_var_integer (ptr_item, "timeout", HOOK_PROCESS(ptr_hook, timeout)))
return 0;
if (!infolist_new_var_integer (ptr_item, "child_stdout_read", HOOK_PROCESS(ptr_hook, child_stdout_read)))
if (!infolist_new_var_integer (ptr_item, "child_read_stdout", HOOK_PROCESS(ptr_hook, child_read[HOOK_PROCESS_STDOUT])))
return 0;
if (!infolist_new_var_integer (ptr_item, "child_stdout_write", HOOK_PROCESS(ptr_hook, child_stdout_write)))
if (!infolist_new_var_integer (ptr_item, "child_write_stdout", HOOK_PROCESS(ptr_hook, child_write[HOOK_PROCESS_STDOUT])))
return 0;
if (!infolist_new_var_integer (ptr_item, "child_stderr_read", HOOK_PROCESS(ptr_hook, child_stderr_read)))
if (!infolist_new_var_integer (ptr_item, "child_read_stderr", HOOK_PROCESS(ptr_hook, child_read[HOOK_PROCESS_STDERR])))
return 0;
if (!infolist_new_var_integer (ptr_item, "child_stderr_write", HOOK_PROCESS(ptr_hook, child_stderr_write)))
if (!infolist_new_var_integer (ptr_item, "child_write_stderr", HOOK_PROCESS(ptr_hook, child_write[HOOK_PROCESS_STDERR])))
return 0;
if (!infolist_new_var_integer (ptr_item, "child_pid", HOOK_PROCESS(ptr_hook, child_pid)))
return 0;
if (!infolist_new_var_pointer (ptr_item, "hook_fd_stdout", HOOK_PROCESS(ptr_hook, hook_fd_stdout)))
if (!infolist_new_var_pointer (ptr_item, "hook_fd_stdout", HOOK_PROCESS(ptr_hook, hook_fd[HOOK_PROCESS_STDOUT])))
return 0;
if (!infolist_new_var_pointer (ptr_item, "hook_fd_stderr", HOOK_PROCESS(ptr_hook, hook_fd_stderr)))
if (!infolist_new_var_pointer (ptr_item, "hook_fd_stderr", HOOK_PROCESS(ptr_hook, hook_fd[HOOK_PROCESS_STDERR])))
return 0;
if (!infolist_new_var_pointer (ptr_item, "hook_timer", HOOK_PROCESS(ptr_hook, hook_timer)))
return 0;
@ -3225,13 +3303,13 @@ hook_print_log ()
log_printf (" callback. . . . . . . : 0x%lx", HOOK_PROCESS(ptr_hook, callback));
log_printf (" command . . . . . . . : '%s'", HOOK_PROCESS(ptr_hook, command));
log_printf (" timeout . . . . . . . : %d", HOOK_PROCESS(ptr_hook, timeout));
log_printf (" child_stdout_read . . : %d", HOOK_PROCESS(ptr_hook, child_stdout_read));
log_printf (" child_stdout_write. . : %d", HOOK_PROCESS(ptr_hook, child_stdout_write));
log_printf (" child_stderr_read . . : %d", HOOK_PROCESS(ptr_hook, child_stderr_read));
log_printf (" child_stderr_write. . : %d", HOOK_PROCESS(ptr_hook, child_stderr_write));
log_printf (" child_read[stdout]. . : %d", HOOK_PROCESS(ptr_hook, child_read[HOOK_PROCESS_STDOUT]));
log_printf (" child_write[stdout] . : %d", HOOK_PROCESS(ptr_hook, child_write[HOOK_PROCESS_STDOUT]));
log_printf (" child_read[stderr]. . : %d", HOOK_PROCESS(ptr_hook, child_read[HOOK_PROCESS_STDERR]));
log_printf (" child_write[stderr] . : %d", HOOK_PROCESS(ptr_hook, child_write[HOOK_PROCESS_STDERR]));
log_printf (" child_pid . . . . . . : %d", HOOK_PROCESS(ptr_hook, child_pid));
log_printf (" hook_fd_stdout. . . . : 0x%lx", HOOK_PROCESS(ptr_hook, hook_fd_stdout));
log_printf (" hook_fd_stderr. . . . : 0x%lx", HOOK_PROCESS(ptr_hook, hook_fd_stderr));
log_printf (" hook_fd[stdout] . . . : 0x%lx", HOOK_PROCESS(ptr_hook, hook_fd[HOOK_PROCESS_STDOUT]));
log_printf (" hook_fd[stderr] . . . : 0x%lx", HOOK_PROCESS(ptr_hook, hook_fd[HOOK_PROCESS_STDERR]));
log_printf (" hook_timer. . . . . . : 0x%lx", HOOK_PROCESS(ptr_hook, hook_timer));
}
break;

View File

@ -68,6 +68,11 @@ enum t_hook_type
#define HOOK_FD_FLAG_WRITE 2
#define HOOK_FD_FLAG_EXCEPTION 4
/* constants for hook process */
#define HOOK_PROCESS_STDOUT 0
#define HOOK_PROCESS_STDERR 1
#define HOOK_PROCESS_BUFFER_SIZE 65536
/* macros to access hook specific data */
#define HOOK_COMMAND(hook, var) (((struct t_hook_command *)hook->hook_data)->var)
#define HOOK_COMMAND_RUN(hook, var) (((struct t_hook_command_run *)hook->hook_data)->var)
@ -180,14 +185,13 @@ struct t_hook_process
t_hook_callback_process *callback; /* process callback (after child end)*/
char *command; /* command executed by child */
long timeout; /* timeout (ms) (0 = no timeout) */
int child_stdout_read; /* to read data in pipe from child */
int child_stdout_write; /* to write data in pipe for child */
int child_stderr_read; /* to read data in pipe from child */
int child_stderr_write; /* to write data in pipe for child */
int child_read[2]; /* to read data in pipe from child */
int child_write[2]; /* to write data in pipe for child */
pid_t child_pid; /* pid of child process */
struct t_hook *hook_fd_stdout; /* hook fd for stdout */
struct t_hook *hook_fd_stderr; /* hook fd for stderr */
struct t_hook *hook_fd[2]; /* hook fd for stdout/stderr */
struct t_hook *hook_timer; /* timer to check if child has died */
char *buffer[2]; /* buffers for child stdout/stderr */
int buffer_size[2]; /* size of child stdout/stderr */
};
/* hook connect */