Re: [PATCH 3/3] parse: replace atoi() with strtoul_ui() and strtol_i()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Usman

On 15/10/2024 16:17, Usman Akinyemi wrote:
On Mon, Oct 14, 2024 at 6:36 PM <phillip.wood123@xxxxxxxxx> wrote:

On 14/10/2024 17:26, Usman Akinyemi wrote:
On Mon, Oct 14, 2024 at 4:13 PM Usman Akinyemi
On Mon, Oct 14, 2024 at 2:55 PM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
I got this from a leftoverbit which the main issue was reported as
bug. https://public-inbox.org/git/CAC4O8c-nuOTS=a0sVp1603KaM2bZjs+yNZzdAaa5CGTNGFE7hQ@xxxxxxxxxxxxxx/
For the test, I should have the test as another patch right ?

In general you should add tests in the same commit as the code changes
that they test. In this instance I think you want to split this patch
into three, one patch for git-daemon, one for imap-send and one for the
merge marker config changes. Each patch should have a commit message
explaining the changes and whether they change the behavior of the code
(for example rejecting non-numbers) and add some tests. Note that I
don't think it is possible to test the imap-send changes but the other
two should be easy enough. The tests should be added to one of the
existing test files that are testing the code being changed.

Hello, thanks for this, I was working on this and I need help. For the
merge-ll.c,
I noticed that the check->items[0].value were already checked to
ensure they do not contain letters in them.
         if (check->items[1].value) {
                 marker_size = atoi(check->items[1].value);
                 if (strtol_i(check->items[1].value, 10, &marker_size))
                         die("invalid marker-size expecting an integer");
                 if (marker_size <= 0)
                         marker_size = DEFAULT_CONFLICT_MARKER_SIZE

error: option `marker-size' expects a numerical value
not ok 38 - merge without conflict wrong marker-size
#
# cp new1.txt test.txt &&
# test_must_fail git merge-file -p --marker-size=1a test.txt orig.txt
new2.txt 2>error &&
# cat error &&
#     grep "invalid" error

It would be better to check for the error message with test_cmp or at least grep for a longer phrase so we're sure the error message is the one we think we should be getting.

#
I grepped the error message and I noticed that the message is gotten
from parse-options.c and it ensures that the arg is negative. How to
proceed in such a case ?

The code you're changing parses the conflict-marker-size attribute so you need to set up a .gitattributes file with an invalid marker size and then run "git merge" or "git cherry-pick"

Best Wishes

Phillip

Also, for the daemon.c I am finding
it hard to get the exact test file to add the new test.

Thank you.
Usman Akinyemi


Thanks.
Also, do I need to add the reference which mentions the leftoverbit in
the commit message?

I'm not sure that's necessary so long as you explain the reason for the
changes in the commit message.


Best Wishes

Phillip







[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux