On Mon, Oct 21, 2024 at 4:34 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
>
> On Mon, Oct 21, 2024 at 02:24:38PM +0000, Usman Akinyemi wrote:
> > On Mon, Oct 21, 2024 at 2:01 PM Patrick Steinhardt <ps@xxxxxx> wrote:
> > >
> > > On Fri, Oct 18, 2024 at 01:52:59PM +0000, Usman Akinyemi via GitGitGadget wrote:
> > > > From: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx>
> > > >
> > > > Replaced atoi() with strtol_i() for parsing conflict-marker-size to
> > > > improve error handling. Invalid values, such as those containing letters
> > > > now trigger a clear error message.
> > > > Updated the test to verify invalid input handling.
> > >
> > > When starting a new paragraph we typically have an empty line between
> > > the paragraphs. We also tend to write commit messages as if instructing
> > > the code to change. So instead of "Replaced atoi() with..." you'd say
> > > "Replace atoi() with", and instead of "Updated the test...", you'd say
> > > "Update the test ...".
> > >
> > > The same applies to your other commits, as well.
>
> Thanks for noting, Patrick.
>
> > > These are a bit curious. As your test demonstrates, we retrieve the
> > > values from the "gitattributes" file. And given that the file tends to be
> > > checked into the repository, you can now basically break somebody elses
> > > commands by having an invalid value in there.
> > >
> > > That makes me think that we likely shouldn't die here. We may print a
> > > warning, but other than that we should likely continue and use the
> > > DEFAULT_CONFLICT_MARKER_SIZE.
> > >
> >
> > Ohh, I understand. Philip suggested this. For the warning, will I just
> > use printf statement or what function to print the statement ?
> > Also, how do I test the print warning statement ?
>
> You can use warning() instead of die(), which will also print the
> message to stderr. You can redirect stderr to a separate file in your
> test, and then grep or test_grep that to ensure that you see the warning
> message.
>
> These messages should also be marked for translation (with `_()`), so
> the result will look something like:
>
> if (strtol_i(check->items[0].value, 10, &marker_size))
> warning(_("invalid marker-size '%s', expecting an integer"),
> check->items[0].value);
Hi Taylor, when I try to use this warning(_, I was getting some error
In the editor
warning(_("invalid marker-size '%s', expecting an integer"),
check->items[1].value); Incompatible integer to pointer conversion
passing 'int' to parameter of type 'const char *'
while I tried run make
erge-ll.c: In function ‘ll_merge’:
merge-ll.c:432:33: error: implicit declaration of function ‘_’
[-Wimplicit-function-declaration]
432 | warning(_("invalid marker-size '%s',
expecting an integer"), check->items[1].value);
| ^
merge-ll.c:432:33: error: passing argument 1 of ‘warning’ makes
pointer from integer without a cast [-Wint-conversion]
432 | warning(_("invalid marker-size '%s',
expecting an integer"), check->items[1].value);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| int
In file included from merge-ll.c:9:
git-compat-util.h:691:26: note: expected ‘const char *’ but argument
is of type ‘int’
691 | void warning(const char *err, ...) __attribute__((format
(printf, 1, 2)));
| ~~~~~~~~~~~~^~~
>
> Thanks,
> Taylor
