| Message ID | 76688ed2cc20031d70823d9f5d214f42b3bd1409.1707501064.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | column: disallow negative padding | expand |
I forgot tests.
On Sat, Feb 10, 2024 at 1:46 AM Kristoffer Haugsbakk <code@khaugsbakk.name> wrote: > I forgot tests. You presumably also wanted the `_` here for gettext-ing: > + die("%s: argument must be a non-negative integer", "padding"); Chris
On Sat, Feb 10, 2024, at 10:48, Chris Torek wrote: > On Sat, Feb 10, 2024 at 1:46 AM Kristoffer Haugsbakk > <code@khaugsbakk.name> wrote: >> I forgot tests. > > You presumably also wanted the `_` here for gettext-ing: > >> + die("%s: argument must be a non-negative integer", "padding"); > > Chris Yeah, thanks. You probably saved me a v3. :) Although I failed to notice that the string I stole was just a plain string, not a translation string. And apparently there are no generic “non-negative” translation strings. So I’ll just make a new one. Cheers
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes: > On Sat, Feb 10, 2024, at 10:48, Chris Torek wrote: >> On Sat, Feb 10, 2024 at 1:46 AM Kristoffer Haugsbakk >> <code@khaugsbakk.name> wrote: >>> I forgot tests. >> >> You presumably also wanted the `_` here for gettext-ing: >> >>> + die("%s: argument must be a non-negative integer", "padding"); >> >> Chris > > Yeah, thanks. You probably saved me a v3. :) > > Although I failed to notice that the string I stole was just a plain > string, not a translation string. And apparently there are no generic > “non-negative” translation strings. So I’ll just make a new one. The last time I took a look, I thought there were more than just the single entry point you patched that can feed negative padding into the machinery? Don't you need to cover them as well? Thanks.
On Sun, Feb 11, 2024, at 18:55, Junio C Hamano wrote: > "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes: >> Yeah, thanks. You probably saved me a v3. :) >> >> Although I failed to notice that the string I stole was just a plain >> string, not a translation string. And apparently there are no generic >> “non-negative” translation strings. So I’ll just make a new one. > > The last time I took a look, I thought there were more than just the > single entry point you patched that can feed negative padding into > the machinery? Don't you need to cover them as well? > > Thanks. I’ve incorporated the `column.c` patch you posted in my not-yet-published v2. Hopefully that was it.(? :) ) I’ll take another look. v2 is finished now so maybe I’ll send it out soon.
diff --git a/builtin/column.c b/builtin/column.c index e80218f81f9..82902d149c2 100644 --- a/builtin/column.c +++ b/builtin/column.c @@ -45,6 +45,8 @@ int cmd_column(int argc, const char **argv, const char *prefix) memset(&copts, 0, sizeof(copts)); copts.padding = 1; argc = parse_options(argc, argv, prefix, options, builtin_column_usage, 0); + if (copts.padding < 0) + die("%s: argument must be a non-negative integer", "padding"); if (argc) usage_with_options(builtin_column_usage, options); if (real_command || command) {
A negative padding can cause some problems in the memory allocator: • floating point exception • data too large to fit into virtual memory space • OOM Disallow negative padding. Reuse a translation string from `fast-import`. Reported-by: Tiago Pascoal <tiago@pascoal.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> --- builtin/column.c | 2 ++ 1 file changed, 2 insertions(+)