[WIP] Support arbitrary .bin map filenames instead of map_XXX.bin convention #244

eugenevinitsky · 2026-01-04T18:50:24Z

Summary

Builds on #243 to fully remove the map_XXX.bin naming requirement.

Added directory scanning utility to discover .bin files dynamically in C
Maps can now be named anything (e.g., highway.bin, downtown.bin) instead of requiring map_000.bin, map_001.bin, etc.

Test plan

All tests passing:

Verify training works with maps named map_000.bin, map_001.bin (backward compatible)
Verify training works with arbitrarily named .bin files
Verify visualize tool can pick random maps from directory (requires display - uses same C logic as binding.c)
Verify sanity check subdirectory creation works with new approach

$ python tests/test_map_filename_flexibility.py
============================================================
Running map filename flexibility tests
============================================================
Testing standard map_000.bin naming...
  Standard naming test passed!
Testing arbitrary map naming...
  Arbitrary naming test passed!
Testing single arbitrary map...
  Single arbitrary map test passed!
Testing empty directory error handling...
  Empty directory test passed (correct error raised)!
Testing non-existent directory error handling...
  Non-existent directory test passed (correct error raised)!
Testing sanity subdirectory creation logic...
  Sanity subdirectory creation test passed!
============================================================
Test Results:
  Standard naming: PASSED
  Arbitrary naming: PASSED
  Single arbitrary map: PASSED
  Empty directory error: PASSED
  Non-existent directory error: PASSED
  Sanity subdirectory creation: PASSED
============================================================

🤖 Generated with Claude Code

…0.bin 🤖 Generated with [Claude Code](https://claude.com/claude-code)

greptile-apps · 2026-01-04T18:54:06Z

Greptile Summary

Removed the map_XXX.bin naming requirement by implementing dynamic .bin file discovery in C and Python validation layers.

Key changes:

Added scan_map_files() in C to discover and sort .bin files alphabetically for deterministic ordering
Updated binding.c and visualize.c to load maps by index from scanned file list instead of formatted filenames
Python validation now checks for any .bin files instead of requiring map_000.bin
Sanity check refactored to use per-map subdirectories instead of copying to map_000.bin
Comprehensive test coverage validates backward compatibility and arbitrary naming

Issues found:

Missing malloc null checks in both binding.c and visualize.c could cause crashes on memory allocation failure
Code duplication: scan_map_files implementation is duplicated across binding.c and visualize.c

Confidence Score: 3/5

PR has critical memory safety issues that should be fixed before merge
Core functionality is well-tested and backward compatible, but missing malloc null checks in C code create crash risk. The logic is sound and tests pass, but memory allocation failures (though rare) would cause segfaults. Code duplication is a style issue but doesn't affect safety.
Pay close attention to pufferlib/ocean/drive/binding.c and pufferlib/ocean/drive/visualize.c - add null checks after all malloc calls

Important Files Changed

Filename	Overview
pufferlib/ocean/drive/binding.c	Added dynamic .bin file discovery with directory scanning; missing malloc null checks could cause crashes on memory allocation failure
pufferlib/ocean/drive/visualize.c	Duplicated scan_map_files implementation for random map selection; same malloc issues as binding.c

Sequence Diagram

sequenceDiagram
    participant User
    participant Python as drive.py
    participant C as binding.c
    participant FS as File System
    
    User->>Python: Initialize Drive environment
    Python->>Python: Check map_dir exists
    Python->>FS: os.listdir(map_dir)
    FS-->>Python: List of files
    Python->>Python: Filter .bin files
    alt No .bin files found
        Python-->>User: FileNotFoundError
    end
    
    Python->>C: Call my_shared() or my_init()
    C->>C: scan_map_files(map_dir)
    C->>FS: opendir(map_dir)
    FS-->>C: Directory handle
    
    loop First pass
        C->>FS: readdir()
        FS-->>C: Directory entry
        C->>C: Count .bin files
    end
    
    C->>C: Allocate filenames array
    C->>FS: rewinddir()
    
    loop Second pass
        C->>FS: readdir()
        FS-->>C: Directory entry
        C->>C: Build full path, store in array
    end
    
    C->>C: qsort(filenames) for deterministic ordering
    C->>FS: closedir()
    
    alt use_all_maps mode
        loop For each map file
            C->>C: Load map by index
            C->>C: Check for controllable agents
        end
    else random sampling mode
        loop Until num_agents met
            C->>C: Select random map_id
            C->>C: Load map_files.filenames[map_id]
            C->>C: Check for controllable agents
        end
    end
    
    C->>C: free_map_file_list()
    C-->>Python: Environment initialized
    Python-->>User: Ready for training

greptile-apps

_{6 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-04T18:54:02Z

pufferlib/ocean/drive/binding.c

+    }
+
+    // Allocate array
+    result.filenames = malloc(count * sizeof(char *));


logic: Missing null check after malloc. If allocation fails, subsequent code will crash.

Suggested change

result.filenames = malloc(count * sizeof(char *));

result.filenames = malloc(count * sizeof(char *));

if (!result.filenames) {

closedir(dir);

return result;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: pufferlib/ocean/drive/binding.c Line: 44:44 Comment: **logic:** Missing null check after `malloc`. If allocation fails, subsequent code will crash. ```suggestion result.filenames = malloc(count * sizeof(char *)); if (!result.filenames) { closedir(dir); return result; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-04T18:54:03Z

pufferlib/ocean/drive/binding.c

+        if (len > 4 && strcmp(name + len - 4, ".bin") == 0) {
+            // Allocate full path
+            size_t path_len = strlen(map_dir) + 1 + len + 1;
+            result.filenames[idx] = malloc(path_len);


logic: Missing null check after malloc. If allocation fails for individual filename, will cause crash when accessed.

Suggested change

result.filenames[idx] = malloc(path_len);

result.filenames[idx] = malloc(path_len);

if (!result.filenames[idx]) {

// Free previously allocated strings

for (int i = 0; i < idx; i++) {

free(result.filenames[i]);

}

free(result.filenames);

closedir(dir);

result.filenames = NULL;

result.count = 0;

return result;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: pufferlib/ocean/drive/binding.c Line: 56:56 Comment: **logic:** Missing null check after `malloc`. If allocation fails for individual filename, will cause crash when accessed. ```suggestion result.filenames[idx] = malloc(path_len); if (!result.filenames[idx]) { // Free previously allocated strings for (int i = 0; i < idx; i++) { free(result.filenames[i]); } free(result.filenames); closedir(dir); result.filenames = NULL; result.count = 0; return result; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-04T18:54:04Z

pufferlib/ocean/drive/visualize.c

+    }
+
+    // Allocate array
+    result.filenames = malloc(count * sizeof(char *));


logic: Missing null check after malloc. Same issue as in binding.c.

Suggested change

result.filenames = malloc(count * sizeof(char *));

result.filenames = malloc(count * sizeof(char *));

if (!result.filenames) {

closedir(dir);

return result;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: pufferlib/ocean/drive/visualize.c Line: 55:55 Comment: **logic:** Missing null check after `malloc`. Same issue as in `binding.c`. ```suggestion result.filenames = malloc(count * sizeof(char *)); if (!result.filenames) { closedir(dir); return result; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-04T18:54:05Z

pufferlib/ocean/drive/visualize.c

+        if (len > 4 && strcmp(name + len - 4, ".bin") == 0) {
+            // Allocate full path
+            size_t path_len = strlen(map_dir) + 1 + len + 1;
+            result.filenames[idx] = malloc(path_len);


logic: Missing null check after malloc for individual filename allocation.

Suggested change

result.filenames[idx] = malloc(path_len);

result.filenames[idx] = malloc(path_len);

if (!result.filenames[idx]) {

for (int i = 0; i < idx; i++) {

free(result.filenames[i]);

}

free(result.filenames);

closedir(dir);

result.filenames = NULL;

result.count = 0;

return result;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: pufferlib/ocean/drive/visualize.c Line: 67:67 Comment: **logic:** Missing null check after `malloc` for individual filename allocation. ```suggestion result.filenames[idx] = malloc(path_len); if (!result.filenames[idx]) { for (int i = 0; i < idx; i++) { free(result.filenames[i]); } free(result.filenames); closedir(dir); result.filenames = NULL; result.count = 0; return result; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-04T18:54:06Z

pufferlib/ocean/drive/visualize.c

+// Structure to hold discovered map files
+typedef struct {
+    char **filenames;
+    int count;
+} MapFileList;
+
+// Compare function for qsort to sort filenames alphabetically
+static int compare_strings(const void *a, const void *b) {
+    return strcmp(*(const char **)a, *(const char **)b);
+}
+
+// Scan directory for .bin files and return sorted list
+static MapFileList scan_map_files(const char *map_dir) {
+    MapFileList result = {NULL, 0};
+    DIR *dir = opendir(map_dir);
+    if (!dir) {
+        return result;
+    }
+
+    // First pass: count .bin files
+    struct dirent *entry;
+    int count = 0;
+    while ((entry = readdir(dir)) != NULL) {
+        const char *name = entry->d_name;
+        size_t len = strlen(name);
+        if (len > 4 && strcmp(name + len - 4, ".bin") == 0) {
+            count++;
+        }
+    }
+
+    if (count == 0) {
+        closedir(dir);
+        return result;
+    }
+
+    // Allocate array
+    result.filenames = malloc(count * sizeof(char *));
+    result.count = count;
+
+    // Second pass: collect filenames
+    rewinddir(dir);
+    int idx = 0;
+    while ((entry = readdir(dir)) != NULL && idx < count) {
+        const char *name = entry->d_name;
+        size_t len = strlen(name);
+        if (len > 4 && strcmp(name + len - 4, ".bin") == 0) {
+            // Allocate full path
+            size_t path_len = strlen(map_dir) + 1 + len + 1;
+            result.filenames[idx] = malloc(path_len);
+            snprintf(result.filenames[idx], path_len, "%s/%s", map_dir, name);
+            idx++;
+        }
+    }
+    closedir(dir);
+
+    // Sort for deterministic ordering
+    qsort(result.filenames, result.count, sizeof(char *), compare_strings);
+
+    return result;
+}
+
+static void free_map_file_list(MapFileList *list) {
+    if (list->filenames) {
+        for (int i = 0; i < list->count; i++) {
+            free(list->filenames[i]);
+        }
+        free(list->filenames);
+        list->filenames = NULL;
+        list->count = 0;
+    }
+}


style: The scan_map_files, compare_strings, and free_map_file_list functions are duplicated from binding.c

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: pufferlib/ocean/drive/visualize.c Line: 19:89 Comment: **style:** The `scan_map_files`, `compare_strings`, and `free_map_file_list` functions are duplicated from `binding.c` <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

- Add directory scanning utility to discover .bin files dynamically - Update binding.c to scan map directory and load maps by index into sorted list - Update visualize.c to pick random maps from scanned directory - Update pufferl.py sanity check to use per-map subdirectories - Add comprehensive test suite for map filename flexibility 🤖 Generated with [Claude Code](https://claude.com/claude-code)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

…fix/flexible_map_names

Python already validates num_maps against available maps with proper user feedback (error or warning). The C-side check was redundant. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

- Python builds sorted map_files list once, passes to C binding - C no longer scans directory independently (removes race condition risk) - my_shared() accepts map_files list instead of map_dir - my_init() accepts map_path instead of map_dir + map_id - Fix memory leaks in visualize.c error paths - Rename allow_map_resampling -> allow_fewer_maps for clarity 🤖 Generated with [Claude Code](https://claude.com/claude-code)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

eugenevinitsky · 2026-01-04T22:39:22Z

@l1onh3art88 @daphne-cornelisse reason for doing this is so we can keep the CARLA maps with the same names

daphne-cornelisse · 2026-01-04T22:48:13Z

Fine with me, but feels like a lot of added code for a relatively arbitrary change. Might be missing something

daphne-cornelisse · 2026-01-04T22:49:58Z

@l1onh3art88 @daphne-cornelisse reason for doing this is so we can keep the CARLA maps with the same names

Missed this. So you want the files to be e.g., womd_0000, womd_0001, carla_000, etc. ?

That makes sense

eugenevinitsky · 2026-01-05T01:47:38Z

oh no they can be anything now! town_01.bin, highway.bin, whatever

eugenevinitsky · 2026-01-05T01:48:13Z

I was annoyed that I was naming carla towns map_001.bin and then not being sure what it actually was once I had forgotten the mapping

eugenevinitsky · 2026-01-05T01:49:04Z

If you remove the tests (optional) and the changes to visualize.c (also optional) it's not a lot of code

eugenevinitsky · 2026-01-05T02:20:49Z

@daphne-cornelisse point taken, I've removed quite a bit of code to make this more compact

- Revert visualize.c directory scanning (use 2.0 base) - Python now picks random map from env_cfg.map_dir - Ensures --env.map-dir propagates to visualizer via --map-name 🤖 Generated with [Claude Code](https://claude.com/claude-code)

daphne-cornelisse · 2026-01-05T14:12:34Z

@daphne-cornelisse point taken, I've removed quite a bit of code to make this more compact

Nice! much better

Check for any .bin files in map directory instead of requiring map_00…

986ceae

…0.bin 🤖 Generated with [Claude Code](https://claude.com/claude-code)

greptile-apps bot reviewed Jan 4, 2026

View reviewed changes

eugenevinitsky force-pushed the fix/flexible_map_names branch 2 times, most recently from 6885336 to 9e5c0c9 Compare January 4, 2026 18:59

eugenevinitsky force-pushed the fix/flexible_map_names branch from 9e5c0c9 to 4a78e45 Compare January 4, 2026 20:32

eugenevinitsky added 2 commits January 4, 2026 15:57

Fix clang-format styling in map_utils.h

6ededaa

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Merge branch '3.0' of https://github.com/Emerge-Lab/PufferDrive into …

fd6f19e

…fix/flexible_map_names

eugenevinitsky requested a review from l1onh3art88 January 4, 2026 21:24

eugenevinitsky changed the title ~~Support arbitrary .bin map filenames instead of map_XXX.bin convention~~ [WIP] Support arbitrary .bin map filenames instead of map_XXX.bin convention Jan 4, 2026

eugenevinitsky added 4 commits January 4, 2026 16:29

Remove redundant num_maps clamping in C binding

20e5e48

Python already validates num_maps against available maps with proper user feedback (error or warning). The C-side check was redundant. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Remove unnecessary comment

2077a20

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Move scan_map_files into visualize.c, delete map_utils.h

2422329

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Delete tests/test_map_filename_flexibility.py

6d2f133

eugenevinitsky force-pushed the fix/flexible_map_names branch from 468439a to f26c600 Compare January 5, 2026 02:24

eugenevinitsky force-pushed the fix/flexible_map_names branch from f26c600 to 44244a0 Compare January 5, 2026 02:27

eugenevinitsky added 2 commits January 4, 2026 21:47

Update utils.py

62957e3

Update utils.py

0afc4b2

-            result.filenames[idx] = malloc(path_len);
+            result.filenames[idx] = malloc(path_len);
+            if (!result.filenames[idx]) {
+                // Free previously allocated strings
+                for (int i = 0; i < idx; i++) {
+                    free(result.filenames[i]);
+                }
+                free(result.filenames);
+                closedir(dir);
+                result.filenames = NULL;
+                result.count = 0;
+                return result;
+            }

[WIP] Support arbitrary .bin map filenames instead of map_XXX.bin convention #244

Are you sure you want to change the base?

[WIP] Support arbitrary .bin map filenames instead of map_XXX.bin convention #244

Uh oh!

Conversation

eugenevinitsky commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

greptile-apps bot commented Jan 4, 2026

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

eugenevinitsky commented Jan 4, 2026

Uh oh!

daphne-cornelisse commented Jan 4, 2026

Uh oh!

daphne-cornelisse commented Jan 4, 2026

Uh oh!

eugenevinitsky commented Jan 5, 2026

Uh oh!

eugenevinitsky commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eugenevinitsky commented Jan 5, 2026

Uh oh!

eugenevinitsky commented Jan 5, 2026

Uh oh!

daphne-cornelisse commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eugenevinitsky commented Jan 4, 2026 •

edited

Loading

eugenevinitsky commented Jan 5, 2026 •

edited

Loading