summaryrefslogtreecommitdiff
path: root/degesch.c
diff options
context:
space:
mode:
authorPřemysl Janouch <p.janouch@gmail.com>2018-01-08 21:53:42 +0100
committerPřemysl Janouch <p.janouch@gmail.com>2018-01-08 22:19:23 +0100
commitb4507b56afcdcdcc443b5a838aaadde7e62ed520 (patch)
tree260acdf47e63708b10236c90b1cbd5256ce39a38 /degesch.c
parentbf6d507bb2c4a353461cca9706da764ab1c246d8 (diff)
downloadxK-b4507b56afcdcdcc443b5a838aaadde7e62ed520.tar.gz
xK-b4507b56afcdcdcc443b5a838aaadde7e62ed520.tar.xz
xK-b4507b56afcdcdcc443b5a838aaadde7e62ed520.zip
degesch: thorough review, no functional changes
Diffstat (limited to 'degesch.c')
-rw-r--r--degesch.c81
1 files changed, 61 insertions, 20 deletions
diff --git a/degesch.c b/degesch.c
index ad4c33e..3ac042d 100644
--- a/degesch.c
+++ b/degesch.c
@@ -1,7 +1,7 @@
/*
* degesch.c: the experimental IRC client
*
- * Copyright (c) 2015 - 2017, Přemysl Janouch <p.janouch@gmail.com>
+ * Copyright (c) 2015 - 2018, Přemysl Janouch <p.janouch@gmail.com>
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -2042,7 +2042,6 @@ struct app_context
struct buffer *current_buffer; ///< The current buffer
struct buffer *last_buffer; ///< Last used buffer
- // TODO: make buffer names fully unique like weechat does
struct str_map buffers_by_name; ///< Buffers by name
unsigned backlog_limit; ///< Limit for buffer lines
@@ -2096,6 +2095,7 @@ filter_color_cube_for_acceptable_nick_colors (size_t *len)
size_t len_counter = 0;
for (int x = 0; x < 6 * 6 * 6; x++)
{
+ // FIXME this isn't exactly right, the values aren't linear
int r = x / 36;
int g = (x / 6) % 6;
int b = (x % 6);
@@ -3122,6 +3122,8 @@ static const int g_mirc_to_terminal[] =
[MIRC_L_GRAY] = COLOR_256 (WHITE, 252),
};
+// TODO: support more colors, see https://modern.ircdocs.horse/formatting.html
+// + http://anti.teamidiot.de/static/nei/*/extended_mirc_color_proposal.html
static const char *
formatter_parse_mirc_color (struct formatter *self, const char *s)
{
@@ -3778,7 +3780,7 @@ static void
buffer_line_write_time (struct formatter *f, struct buffer_line *line,
FILE *stream, int flush_opts)
{
- // Normal timestamps don't include the date, this way the user won't be
+ // Normal timestamps don't include the date, make sure the user won't be
// confused as to when an event has happened
buffer_update_time (f->ctx, line->when, stream, flush_opts);
@@ -4003,7 +4005,6 @@ buffer_get_log_path (struct buffer *buffer)
(void) mkdir_with_parents (path.str, NULL);
- // TODO: make sure global and server buffers don't collide with filenames
str_append_c (&path, '/');
make_log_filename (buffer->name, &path);
str_append (&path, ".log");
@@ -4016,6 +4017,7 @@ buffer_open_log_file (struct app_context *ctx, struct buffer *buffer)
if (!ctx->logging || buffer->log_file)
return;
+ // TODO: should we try to reopen files wrt. case mapping?
char *path = buffer_get_log_path (buffer);
if (!(buffer->log_file = fopen (path, "ab")))
log_global_error (ctx, "Couldn't open log file `#s': #l",
@@ -4064,7 +4066,8 @@ buffer_add (struct app_context *ctx, struct buffer *buffer)
buffer_open_log_file (ctx, buffer);
- // In theory this can't cause changes in the prompt
+ // Normally this doesn't cause changes in the prompt but a prompt hook
+ // could decide to show some information for all buffers nonetheless
refresh_prompt (ctx);
}
@@ -4140,7 +4143,7 @@ buffer_print_backlog (struct app_context *ctx, struct buffer *buffer)
free (buffer_name_localized);
}
- // That is, minus the readline prompt
+ // That is, minus the readline prompt (taking at least one line)
int display_limit = MAX (10, g_terminal.lines - 1);
int to_display = 0;
@@ -4176,6 +4179,9 @@ buffer_activate (struct app_context *ctx, struct buffer *buffer)
{
if (ctx->current_buffer == buffer)
return;
+
+ // This is the only place where the unread messages marker
+ // and highlight indicator are reset
if (ctx->current_buffer)
{
ctx->current_buffer->new_messages_count = 0;
@@ -5025,6 +5031,7 @@ irc_process_hooks (struct server *s, char *input)
return NULL;
}
+ // The old input may get freed, so we compare against a hash of it
uint64_t new_hash = siphash_wrapper (input, strlen (input));
if (new_hash != hash)
log_server_debug (s, "#a>= \"#S\"#r", ATTR_JOIN, input);
@@ -5067,7 +5074,9 @@ irc_try_read (struct server *s)
enum socket_io_result result = s->transport->try_read (s);
if (s->read_buffer.len >= (1 << 20))
{
- // XXX: this is stupid; if anything, count it in dependence of time
+ // XXX: this is stupid; if anything, count it in dependence of time;
+ // we could make transport_tls_try_read() limit the immediate amount
+ // of data read like socket_io_try_read() does and remove this check
log_server_error (s, s->buffer,
"The IRC server seems to spew out data frantically");
return SOCKET_IO_ERROR;
@@ -5302,7 +5311,7 @@ transport_tls_init_ctx (struct server *s, SSL_CTX *ssl_ctx, struct error **e)
return false;
}
- // Only inform the user if we're not actually verifying
+ // Just inform the user if we're not actually verifying
log_server_error (s, s->buffer, "#s", error->message);
error_free (error);
}
@@ -5363,7 +5372,7 @@ transport_tls_init (struct server *s, const char *hostname, struct error **e)
// Enable SNI, FWIW; literal IP addresses aren't allowed
struct in6_addr dummy;
- if (!inet_pton (AF_INET, hostname, &dummy)
+ if (!inet_pton (AF_INET, hostname, &dummy)
&& !inet_pton (AF_INET6, hostname, &dummy))
SSL_set_tlsext_host_name (ssl, hostname);
@@ -6035,7 +6044,7 @@ irc_is_highlight (struct server *s, const char *message)
// Special characters allowed in nicknames by RFC 2812: []\`_^{|} and -
// Also excluded from the ASCII: common user channel prefixes: +%@&~
- // XXX: why did I exclude those? It won't match when newbies use them.
+ // XXX: why did I exclude those? It won't match when IRC newbies use them.
const char *delimiters = ",.;:!?()<>/=#$* \t\r\n\v\f\"'";
bool result = false;
@@ -6329,6 +6338,7 @@ irc_handle_sent_privmsg (struct server *s, const struct irc_message *msg)
return;
// This ignores empty messages which we should not normally send
+ // and the server is likely going to reject with an error reply anyway
struct ctcp_chunk *chunks = ctcp_parse (msg->params.vector[1]);
LIST_FOR_EACH (struct ctcp_chunk, iter, chunks)
{
@@ -6367,6 +6377,7 @@ static void
irc_process_sent_message (const struct irc_message *msg, struct server *s)
{
// The server is free to reject even a matching prefix
+ // XXX: even though no prefix should normally be present, this is racy
if (msg->prefix && !irc_is_this_us (s, msg->prefix))
return;
@@ -7135,6 +7146,10 @@ irc_on_registered (struct server *s, const char *nickname)
refresh_prompt (s->ctx);
// XXX: we can also use WHOIS if it's not supported (optional by RFC 2812)
+ // TODO: maybe rather always use RPL_ISUPPORT NICKLEN & USERLEN & HOSTLEN
+ // since we don't seem to follow any subsequent changes in userhost;
+ // unrealircd sends RPL_HOSTHIDDEN (396), which has an optional user part,
+ // and there is also CAP CHGHOST which /may/ send it to ourselves
irc_send (s, "USERHOST %s", s->irc_user->nickname);
// A little hack that reinstates auto-away status when we get disconnected
@@ -7225,7 +7240,7 @@ irc_handle_rpl_namreply (struct server *s, const struct irc_message *msg)
struct channel_user_sort_entry
{
- struct server *s; ///< Server
+ struct server *s; ///< Server (because of the qsort API)
struct channel_user *channel_user; ///< Channel user
};
@@ -8182,7 +8197,10 @@ server_rename (struct app_context *ctx, struct server *s, const char *new_name)
struct buffer *buffer;
while ((buffer = str_map_iter_next (&iter)))
{
- // FIXME: creation of buffer names should be centralized
+ // TODO: creation of buffer names should be centralized -> replace
+ // calls to buffer_rename() and manual setting of buffer names
+ // with something like buffer_autorename() -- just mind the mess
+ // in irc_handle_nick(), which can hopefully be simplified
char *x = NULL;
switch (buffer->type)
{
@@ -8387,6 +8405,7 @@ lua_plugin_log_error
error_free (error);
}
+/// Pop "n" values from the stack into a table, using their indexes as keys
static void
lua_plugin_pack (lua_State *L, int n)
{
@@ -8541,7 +8560,7 @@ LUA_WEAK_DECLARE (channel, XLUA_CHANNEL_METATABLE)
LUA_WEAK_DECLARE (buffer, XLUA_BUFFER_METATABLE)
LUA_WEAK_DECLARE (server, XLUA_SERVER_METATABLE)
-// The global context is kind of fake and don't have any ref-counting,
+// The global context is kind of fake and doesn't have any ref-counting,
// however it's still very much an object
static struct lua_weak_info lua_ctx_info =
{
@@ -9332,8 +9351,13 @@ lua_connection_close (lua_State *L)
if (self->socket_fd != -1)
{
self->closing = true;
+ // NOTE: this seems to do nothing on Linux
(void) shutdown (self->socket_fd, SHUT_RD);
+ // Right now we want to wait until all data is flushed to the socket
+ // and can't call close() here immediately -- a rewrite to use async
+ // would enable the user to await on either :send() or :flush();
+ // a successful send() doesn't necessarily mean anything though
if (!self->write_buffer.len)
lua_connection_discard (self);
}
@@ -9413,6 +9437,8 @@ static bool
lua_connection_invoke_on_error (struct lua_connection *self,
const char *error, struct error **e)
{
+ // XXX: not sure if ignoring errors after :close() is always desired;
+ // code might want to make sure that data are transferred successfully
if (!self->closing
&& lua_connection_cb_lookup (self, "on_error", e)
&& !lua_connection_eat_nil (self))
@@ -9791,8 +9817,13 @@ lua_wait_dial_check (struct lua_wait_channel *wchannel)
lua_State *L = self->super.task->thread;
if (self->connection)
{
- // FIXME: this way the connection can leak, it shouldn't stay in cache
- // automatically all the time but clean itself up on GC
+ // FIXME: this way the connection may leak -- we pass the value to the
+ // task manager on the stack and forget about it but still leave the
+ // connection in the cache. That is because right now, when Lua code
+ // sets up callbacks in the connection object and returns, it might
+ // get otherwise GC'd since nothing else keeps referencing it.
+ // By rewriting lua_connection using async, tasks and wait channels
+ // would hold a reference, allowing us to remove it from the cache.
lua_cache_get (L, self->connection);
lua_pushstring (L, self->hostname);
self->connection = NULL;
@@ -10040,7 +10071,7 @@ lua_plugin_push_struct (lua_State *L, struct lua_plugin *self,
{
if (type == ISPECT_STR)
{
- struct str *s = value;
+ const struct str *s = value;
lua_pushlstring (L, s->str, s->len);
return;
}
@@ -10194,7 +10225,8 @@ lua_plugin_reg_meta (lua_State *L, const char *name, luaL_Reg *fns)
static void
lua_plugin_reg_weak (lua_State *L, struct lua_weak_info *info, luaL_Reg *fns)
{
- // Create a mapping from the object type back to our metadata
+ // Create a mapping from the object type (info->ispect) back to metadata
+ // so that we can figure out what to create from ISPECT_REF fields
lua_pushlightuserdata (L, info);
lua_rawsetp (L, LUA_REGISTRYINDEX, info->ispect);
@@ -10359,7 +10391,8 @@ plugin_load (struct app_context *ctx, const char *name)
struct plugin *plugin = plugin_load_by_name (ctx, name, &e);
if (plugin)
{
- // FIXME: this way the real name isn't available to the plugin on load
+ // FIXME: this way the real name isn't available to the plugin on load,
+ // which has effect on e.g. plugin_config_name()
free (plugin->name);
plugin->name = xstrdup (name);
@@ -10464,6 +10497,7 @@ static char *
maybe_cut_word_from_end (char **s, word_validator_fn validator, void *user_data)
{
// Find the start and end of the last word
+ // Contrary to maybe_cut_word(), we ignore all whitespace at the end
char *start = *s, *end = start + strlen (start);
while (end > start && strchr (WORD_BREAKING_CHARS, end [-1]))
end--;
@@ -11149,7 +11183,9 @@ static bool
handle_command_topic (struct handler_args *a)
{
if (*a->arguments)
- // FIXME: there's no way to unset the topic
+ // FIXME: there's no way to start the topic with whitespace
+ // FIXME: there's no way to unset the topic;
+ // we could adopt the Tcl style of "-switches" with "--" sentinels
irc_send (a->s, "TOPIC %s :%s", a->channel_name, a->arguments);
else
irc_send (a->s, "TOPIC %s", a->channel_name);
@@ -11228,6 +11264,7 @@ mass_channel_mode_mask_list
for (size_t i = 0; i < v.len; i++)
{
char *target = v.vector[i];
+ // TODO: support EXTBAN and leave those alone, too
if (strpbrk (target, "!@*?"))
continue;
@@ -11768,6 +11805,7 @@ handle_command_help (struct handler_args *a)
if (!*a->arguments)
return show_command_list (ctx);
+ // TODO: we should probably also accept commands names with a leading slash
char *command = cut_word (&a->arguments);
for (size_t i = 0; i < N_ELEMENTS (g_command_handlers); i++)
{
@@ -12963,7 +13001,9 @@ on_readline_input (char *line)
add_history (line);
// readline always erases the input line after returning from here,
- // but we don't want that in order to allow correct buffer switching
+ // but we don't want that to happen if the command to be executed
+ // would switch the buffer (we'd keep the already executed command in
+ // the old buffer and delete any input restored from the new buffer)
strv_append_owned (&ctx->pending_input, line);
poller_idle_set (&ctx->input_event);
}
@@ -13450,6 +13490,7 @@ process_mirc_escape (const struct pollfd *fd, struct app_context *ctx)
goto error;
buf->str[++buf->len] = '\0';
+ // XXX: I think this should be global and shared with Readline/libedit
mbstate_t state;
memset (&state, 0, sizeof state);