Skip to content

Fix statistical seed bias, off-by-one range errors, and resource handling bugs#1

Merged
feinorgh merged 3 commits intomasterfrom
copilot/analyze-code-performance
Mar 5, 2026
Merged

Fix statistical seed bias, off-by-one range errors, and resource handling bugs#1
feinorgh merged 3 commits intomasterfrom
copilot/analyze-code-performance

Conversation

Copy link
Contributor

Copilot AI commented Mar 5, 2026

Several correctness and robustness bugs affected the quality of generated numbers and the valid parameter space. The most critical issue caused the RNG to be seeded with only 1 byte despite reading 256 bytes of entropy.

Statistical correctness

  • Seed entropy truncated: mpz_import(seed, 1, 1, 1, ...) imported only the first byte of the 256-byte entropy buffer. Fixed to mpz_import(seed, count, 1, sizeof(char), ...).
  • range was exclusive: Global range was computed as high − low throughout, but the inclusive count high − low + 1 is required for correct uniform sampling and inverse-method sizing. Now set once (correctly) in parse_options().

Off-by-one bugs

  • Upper bound excluded from output: The inverse-method scan loop while (mpz_cmp(tmp, high)) never printed high. Fixed to <= 0.
  • Range validity check too strict: mpz_cmp(arg_count, range) >= 0 rejected valid counts up to the full range size. Fixed to > 0 (after correcting range to be inclusive).
  • invsize undercounted by 1: Was computed from the off-by-one range, causing one fewer exclusion to be generated than needed.

Resource handling

  • Wrong buffer size: malloc(count * sizeof(char*) + 1) allocated 8× the needed bytes on 64-bit. Fixed to malloc(count + 1).
  • Incomplete read() not retried: A single read() from /dev/random may return fewer bytes than requested. Added a retry loop with error and EOF checks.
  • Missing open() and malloc() error checks in init_random().
  • Missing null check for malloc in the inverse path of generate_series(); handles invsize == 0 (full-range generation) safely.
  • mpz_clear(range) called before mpz_init(range) when exiting via -h/-v/--usage. Fixed by moving mpz_init(range) to main().
  • Memory leak in verbose mode: strings from mpz_get_str(NULL, ...) were never freed.

Minor

  • Added break after case 'r' (implicit fallthrough to case '?' was harmless but misleading).
  • Removed unused high parameter from random_integer() since the global range now holds the inclusive count.
  • Suppressed -Wunused-parameter for depth in treeaction() with (void)depth.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits March 5, 2026 13:36
…ity fixes

Co-authored-by: feinorgh <3132980+feinorgh@users.noreply.github.com>
… random.c

Co-authored-by: feinorgh <3132980+feinorgh@users.noreply.github.com>
Copilot AI changed the title [WIP] Analyze code for performance and compatibility improvements Fix statistical seed bias, off-by-one range errors, and resource handling bugs Mar 5, 2026
@feinorgh feinorgh marked this pull request as ready for review March 5, 2026 13:47
@feinorgh feinorgh merged commit 04b31fd into master Mar 5, 2026
@feinorgh feinorgh deleted the copilot/analyze-code-performance branch March 5, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants