git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] am: fix condition check on fseek

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 23, 2024, 16:43 UTC
Message-ID
<xmqqcyku48yq.fsf@gitster.g>
In-Reply-To
<7601dcef-e256-4e20-bbd4-75955e5a45d5@app.fastmail.com>
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
Show 31 quoted lines
> On Sat, Sep 21, 2024, at 17:08, Ruffalo Lavoisier wrote:
>> if fseek() is success, return value is 0
>>
>> Signed-off-by: Ruffalo Lavoisier <RuffaloLavoisier@gmail.com>
>> ---
>>  builtin/am.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index d8875ad402..a7727fd4ea 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -589,7 +589,7 @@ static int is_mail(FILE *fp)
>>  	regex_t regex;
>>  	int ret = 1;
>>
>> -	if (fseek(fp, 0L, SEEK_SET))
>> +	if (!fseek(fp, 0L, SEEK_SET))
>>  		die_errno(_("fseek failed"));
>>
>>  	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
>> --
>> 2.46.1
>
> I don’t get this change? The function returns false on success. true if
> it fails (not zero). You want the program to die if it returns non-zero.
>
> It’s hard to wrap my head around… “false must mean “no errors” ”
>
> If the original code has a bug then I don’t see how git-am(1) could work
> considering it presumably always checks ‘is_mail’.

Yeah, the proposed log message states a correct fact, but it is unclear how that justifies the change in the patch part of the message.

Previous: Kristoffer Haugsbakk
Message 3 of 3 in “am: fix condition check on fseek”
  1. am: fix condition check on fseekRuffalo Lavoisier, Sep 21, 2024
  2. Kristoffer HaugsbakkSep 21, 2024
  3. Junio C HamanoSep 23, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.