rust-cli / rust-cli/config-rs

Floats in configuration probably shouldn't round to integers if theres precision loss

Open
#160 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

E-help-wanted
Dominant language
Rust
Stars
3.2k
Forks
265
Avg merge
2h 42m
Merged PRs (30d)
4

Description

Currently when you Config::get an integer and config has a float specified, the config-rs crate will transparently lose the precision, potentially leading to unexpected behaviour. For instance:

frame_rate = 59.94

will transparently round up to 60, potentially giving wrong video. There are probably many more instances where loss of precision can give very invalid results.

I believe the relevant code is probably this line.

I saw a number of FIXMEs in the codebase around not succeeding when integer wraparound occurs. We most likely can utilize something similar here – if converting results in loss of precision, fail.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/value.rs at the conversion around line 223, then compare the integer wraparound handling and FIXME context in src/de.rs around line 76. Trace how Config::get converts a float to an integer and identify the existing error path to use. Done means conversions that lose fractional precision fail instead of silently rounding, including the 59.94 example.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.