From 229134cf314e6963875509c88fd46676d19bce6d Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Fri, 4 Sep 2020 20:46:57 +0530 Subject: [PATCH] Misc. fixes for issues reported by clang's static analyzer Most of them are false positives. A couple of mem leaks after unlikely failure conditions. --- glfw/x11_window.c | 20 ++++++++++++-------- kittens/choose/main.c | 4 ++-- kittens/choose/vector.h | 5 +++-- kitty/child-monitor.c | 4 ++-- kitty/freetype.c | 10 ++++++++-- kitty/hyperlink.c | 8 ++++---- 6 files changed, 31 insertions(+), 20 deletions(-) diff --git a/glfw/x11_window.c b/glfw/x11_window.c index 1aebae797..c6464e184 100644 --- a/glfw/x11_window.c +++ b/glfw/x11_window.c @@ -422,14 +422,16 @@ static char* convertLatin1toUTF8(const char* source) size_t size = 1; const char* sp; - for (sp = source; *sp; sp++) - size += (*sp & 0x80) ? 2 : 1; + if (source) { + for (sp = source; *sp; sp++) size += (*sp & 0x80) ? 2 : 1; + } char* target = calloc(size, 1); char* tp = target; - for (sp = source; *sp; sp++) - tp += encodeUTF8(tp, *sp); + if (source) { + for (sp = source; *sp; sp++) tp += encodeUTF8(tp, *sp); + } return target; } @@ -771,7 +773,7 @@ static Atom writeTargetToProperty(const XSelectionRequestEvent* request) if (j < formatCount) { - XChangeProperty(_glfw.x11.display, + if (selectionString) XChangeProperty(_glfw.x11.display, request->requestor, targets[i + 1], targets[i], @@ -823,7 +825,7 @@ static Atom writeTargetToProperty(const XSelectionRequestEvent* request) { // The requested target is one we support - XChangeProperty(_glfw.x11.display, + if (selectionString) XChangeProperty(_glfw.x11.display, request->requestor, request->property, request->target, @@ -954,8 +956,10 @@ static const char* getSelectionString(Atom selection) (XPointer) ¬ification)) { monotonic_t time = glfwGetTime(); - if (time - start > s_to_monotonic_t(2ll)) + if (time - start > s_to_monotonic_t(2ll)) { + free(string); return ""; + } waitForX11Event(s_to_monotonic_t(2ll) - (time - start)); } @@ -1518,7 +1522,7 @@ static void processEvent(XEvent *event) count = 3; formats = (Atom*) event->xclient.data.l + 2; } - char **atom_names = calloc(count, sizeof(char**)); + char **atom_names = calloc(count, sizeof(char*)); if (atom_names) { get_atom_names(formats, count, atom_names); diff --git a/kittens/choose/main.c b/kittens/choose/main.c index 85ee5995e..0c22b6696 100644 --- a/kittens/choose/main.c +++ b/kittens/choose/main.c @@ -111,7 +111,7 @@ end: if (job_data[i] && job_data[i]->started) wait_for_thread(threads, i); } } - for (i = 0; i < num_threads; i++) job_data[i] = free_job(job_data[i]); + if (job_data) { for (i = 0; i < num_threads; i++) job_data[i] = free_job(job_data[i]); } free(job_data); free_threads(threads); return ret; @@ -130,7 +130,7 @@ run_search(Options *opts, GlobalData *global, const char * const *lines, const s ALLOC_VEC(text_t, chars, 8192 * 20); if (chars.data == NULL) return 1; ALLOC_VEC(Candidate, candidates, 8192); - if (candidates.data == NULL) { FREE_VEC(chars) return 1; } + if (candidates.data == NULL) { FREE_VEC(chars); return 1; } for (size_t i = 0; i < num_lines; i++) { sz = sizes[i]; diff --git a/kittens/choose/vector.h b/kittens/choose/vector.h index 72413c731..742a8412f 100644 --- a/kittens/choose/vector.h +++ b/kittens/choose/vector.h @@ -28,8 +28,9 @@ #define ENSURE_SPACE(TYPE, vec, amt) \ if (vec.size + amt >= vec.capacity) { \ vec.capacity = MAX(vec.capacity * 2, vec.size + amt); \ - vec.data = (TYPE*)realloc(vec.data, sizeof(TYPE) * vec.capacity); \ - if (vec.data == NULL) { REPORT_OOM; ret = 1; break; } \ + void *temp = realloc(vec.data, sizeof(TYPE) * vec.capacity); \ + if (temp == NULL) { REPORT_OOM; ret = 1; free(vec.data); vec.data = NULL; vec.size = 0; vec.capacity = 0; break; } \ + else vec.data = temp; \ } #define NEXT(vec) (vec.data[vec.size]) diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index 2aa7adf61..d9f990bd6 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -236,7 +236,7 @@ schedule_write_to_child(unsigned long id, unsigned int num, ...) { va_list ap; va_start(ap, num); for (unsigned int i = 0; i < num; i++) { - data = va_arg(ap, const char*); + va_arg(ap, const char*); sz += va_arg(ap, size_t); } va_end(ap); @@ -1539,7 +1539,7 @@ send_response(id_type peer_id, const char *msg, size_t msg_sz) { peer->write.capacity += msg_sz; } else fatal("Out of memory"); } - memcpy(peer->write.data + peer->write.used, msg, msg_sz); + if (msg) memcpy(peer->write.data + peer->write.used, msg, msg_sz); peer->write.used += msg_sz; } wakeup = true; diff --git a/kitty/freetype.c b/kitty/freetype.c index 3917cb894..47eaad0d3 100644 --- a/kitty/freetype.c +++ b/kitty/freetype.c @@ -589,10 +589,16 @@ render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *inf if (!render_color_bitmap(self, info[i].codepoint, &bm, cell_width, cell_height, num_cells, baseline)) { if (PyErr_Occurred()) PyErr_Print(); *was_colored = false; - if (!render_bitmap(self, info[i].codepoint, &bm, cell_width, cell_height, num_cells, bold, italic, true, fg)) return false; + if (!render_bitmap(self, info[i].codepoint, &bm, cell_width, cell_height, num_cells, bold, italic, true, fg)) { + free_processed_bitmap(&bm); + return false; + } } } else { - if (!render_bitmap(self, info[i].codepoint, &bm, cell_width, cell_height, num_cells, bold, italic, true, fg)) return false; + if (!render_bitmap(self, info[i].codepoint, &bm, cell_width, cell_height, num_cells, bold, italic, true, fg)) { + free_processed_bitmap(&bm); + return false; + } } x_offset = x + (float)positions[i].x_offset / 64.0f; y = (float)positions[i].y_offset / 64.0f; diff --git a/kitty/hyperlink.c b/kitty/hyperlink.c index b9e4ea2bc..88d2432d4 100644 --- a/kitty/hyperlink.c +++ b/kitty/hyperlink.c @@ -40,7 +40,7 @@ clear_pool(HyperLinkPool *pool) { HyperLinkEntry *tmp, *s; HASH_ITER(hh, pool->hyperlinks, s, tmp) { HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); + free_hyperlink_entry(s); s = NULL; } pool->max_link_id = 0; } @@ -88,7 +88,7 @@ screen_garbage_collect_hyperlink_pool(Screen *screen) { pool->max_link_id = MAX(pool->max_link_id, s->id); } else { HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); + free_hyperlink_entry(s); s = NULL; } } } else clear_pool(pool); @@ -118,12 +118,12 @@ get_id_for_hyperlink(Screen *screen, const char *id, const char *url) { } hyperlink_id_type new_id = 0; if (pool->num_of_adds_since_garbage_collection >= MAX_ADDS_BEFORE_GC) screen_garbage_collect_hyperlink_pool(screen); - if (pool->max_link_id >= HYPERLINK_MAX_NUMBER) { + if (pool->max_link_id >= HYPERLINK_MAX_NUMBER && pool->hyperlinks) { log_error("Too many hyperlinks, discarding oldest, this means some hyperlinks might be incorrect"); new_id = pool->hyperlinks->id; HyperLinkEntry *s = pool->hyperlinks; HASH_DEL(pool->hyperlinks, s); - free_hyperlink_entry(s); + free_hyperlink_entry(s); s = NULL; } s = malloc(sizeof(HyperLinkEntry)); if (!s) fatal("Out of memory");