Repository navigation
feat: resolve real Docker container names in CONTAINER column #2125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /* | ||
| htop - DockerMgr.c | ||
| Retrieves running Docker container names via the Docker API over the unix socket. | ||
| Released under the GNU GPLv2+, see the COPYING file | ||
| in the source distribution for its full text. | ||
| */ | ||
|
|
||
| #include "config.h" // IWYU pragma: keep | ||
|
|
||
| #include "linux/DockerMgr.h" | ||
|
|
||
| #include <stddef.h> | ||
| #include <stdio.h> | ||
| #include <string.h> | ||
| #include <sys/socket.h> | ||
| #include <sys/un.h> | ||
| #include <unistd.h> | ||
|
|
||
| #include "Hashtable.h" | ||
| #include "XUtils.h" | ||
|
|
||
| #define DOCKER_SOCKET "/var/run/docker.sock" | ||
| #define RESPONSE_SIZE 262144 | ||
| #define MAX_CONTAINER_NAME_LEN 25 | ||
|
|
||
| /* ponytail: simplest possible cache; refresh by dropping cache on long IDs is out of scope */ | ||
| static Hashtable* nameCache = NULL; | ||
|
|
||
| static unsigned int hashStr(const char* s) { | ||
| unsigned int h = 5381; | ||
| while (*s) | ||
| h = (h * 33) ^ (unsigned char)*s++; | ||
| return h; | ||
| } | ||
|
|
||
| static Hashtable* getCache(void) { | ||
| if (!nameCache) | ||
| nameCache = Hashtable_new(16, true); | ||
| return nameCache; | ||
| } | ||
|
|
||
| static char* queryName(const char* id) { | ||
| int fd = socket(AF_UNIX, SOCK_STREAM, 0); | ||
| if (fd < 0) | ||
| return NULL; | ||
|
|
||
| struct sockaddr_un addr; | ||
| memset(&addr, 0, sizeof(addr)); | ||
| addr.sun_family = AF_UNIX; | ||
| snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", DOCKER_SOCKET); | ||
|
|
||
| if (connect(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { | ||
| close(fd); | ||
| return NULL; | ||
| } | ||
|
|
||
| char req[256]; | ||
| int reqLen = snprintf(req, sizeof(req), | ||
| "GET /containers/%s/json HTTP/1.1\r\n" | ||
| "Host: localhost\r\n" | ||
| "Accept: application/json\r\n" | ||
| "Connection: close\r\n\r\n", id); | ||
| if (reqLen <= 0 || reqLen >= (int)sizeof(req) || write(fd, req, (size_t)reqLen) != reqLen) { | ||
| close(fd); | ||
| return NULL; | ||
| } | ||
|
|
||
| char buf[RESPONSE_SIZE]; | ||
| size_t used = 0; | ||
| ssize_t rd; | ||
| while (used < sizeof(buf) - 1 && (rd = read(fd, buf + used, sizeof(buf) - 1 - used)) > 0) | ||
| used += (size_t)rd; | ||
| close(fd); | ||
|
|
||
| if (used == 0) | ||
| return NULL; | ||
| buf[used] = '\0'; | ||
|
|
||
| char* key = strstr(buf, "\"Name\":\""); | ||
| if (!key) | ||
| return NULL; | ||
| char* val = key + strlen("\"Name\":\""); | ||
| if (*val == '/') | ||
| val++; | ||
| char* end = strchr(val, '"'); | ||
| if (!end) | ||
| return NULL; | ||
| return xStrndup(val, (size_t)(end - val)); | ||
| } | ||
|
|
||
|
Comment on lines
+42
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,125p' linux/DockerMgr.c
printf '\n-- socket timeout/nonblocking patterns --\n'
rg -n --glob '*.[ch]' 'SO_RCVTIMEO|SO_SNDTIMEO|poll\(|select\(|fcntl\(.*O_NONBLOCK|connect\(' linux | head -120Repository: htop-dev/htop Length of output: 3102 Bound the complete Docker socket exchange
The stack-buffer and negative-cache suggestions are separate concerns and are omitted from this correction. |
||
| char* DockerMgr_getContainerName(const char* id) { | ||
| Hashtable* cache = getCache(); | ||
| ht_key_t key = hashStr(id); | ||
|
|
||
| char* cached = Hashtable_get(cache, key); | ||
| if (cached) | ||
| return xStrdup(cached); | ||
|
|
||
| char* name = queryName(id); | ||
| if (name) { | ||
| /* Limit the container name shown in the CONTAINER column to 25 characters */ | ||
| char* shortName = xStrndup(name, MAX_CONTAINER_NAME_LEN); | ||
| free(name); | ||
| Hashtable_put(cache, key, shortName); | ||
| return xStrdup(shortName); | ||
| } | ||
| return NULL; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #ifndef HEADER_DockerMgr | ||
| #define HEADER_DockerMgr | ||
| /* | ||
| htop - DockerMgr.h | ||
| Retrieves running Docker container names via the Docker API over the unix socket. | ||
| Released under the GNU GPLv2+, see the COPYING file | ||
| in the source distribution for its full text. | ||
| */ | ||
|
|
||
| char* DockerMgr_getContainerName(const char* id); | ||
|
|
||
| #endif /* HEADER_DockerMgr */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: htop-dev/htop
Length of output: 3258
🏁 Script executed:
Repository: htop-dev/htop
Length of output: 13313
🏁 Script executed:
Repository: htop-dev/htop
Length of output: 7254
🏁 Script executed:
Repository: htop-dev/htop
Length of output: 6938
Use a collision-safe cache key for Docker IDs.
hashStrreduces each 12-character Docker ID to a 32-bitht_key_t. Distinct IDs can therefore produce the same key.Hashtable_getcompares only that key, so the second ID can receive the first ID's cached name.Use a representation that preserves all 12 hexadecimal characters. Parsing into an integer requires widening
ht_key_tbeyond its current 32-bit type. Otherwise, retain the full ID and handle hash collisions before returning or replacing entries.The owner-semantics warning does not apply:
nameCachealready usesHashtable_new(16, true), and replacement values are freed under that setting.