* [PATCH] am: fix condition check on fseek
@ 2024-09-21 15:08 Ruffalo Lavoisier
2024-09-21 19:52 ` Kristoffer Haugsbakk
0 siblings, 1 reply; 3+ messages in thread
From: Ruffalo Lavoisier @ 2024-09-21 15:08 UTC (permalink / raw)
To: git; +Cc: Ruffalo Lavoisier, Stephan Beyer, Junio C Hamano, Jeff King
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(®ex, header_regex, REG_NOSUB | REG_EXTENDED))
--
2.46.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] am: fix condition check on fseek
2024-09-21 15:08 [PATCH] am: fix condition check on fseek Ruffalo Lavoisier
@ 2024-09-21 19:52 ` Kristoffer Haugsbakk
2024-09-23 16:43 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Kristoffer Haugsbakk @ 2024-09-21 19:52 UTC (permalink / raw)
To: Ruffalo Lavoisier; +Cc: Stephan Beyer, Junio C Hamano, Jeff King, git
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(®ex, 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’.
--
Kristoffer
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] am: fix condition check on fseek
2024-09-21 19:52 ` Kristoffer Haugsbakk
@ 2024-09-23 16:43 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2024-09-23 16:43 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Ruffalo Lavoisier, Stephan Beyer, Jeff King, git
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> 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(®ex, 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-23 16:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-21 15:08 [PATCH] am: fix condition check on fseek Ruffalo Lavoisier
2024-09-21 19:52 ` Kristoffer Haugsbakk
2024-09-23 16:43 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).