{"thread":{"id":"62166","subject":"[PATCH] am: fix condition check on fseek","startedAt":"2024-09-21T15:09:05Z","lastAt":"2024-09-23T16:43:12Z","messageCount":3,"participants":["Ruffalo Lavoisier","Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"503194","messageId":"20240921150855.31574-1-RuffaloLavoisier@gmail.com","threadId":"62166","inReplyTo":null,"subject":"[PATCH] am: fix condition check on fseek","fromName":"Ruffalo Lavoisier","fromEmail":"ruffalolavoisier@gmail.com","sentAt":"2024-09-21T15:08:55Z","receivedAt":"2024-09-21T15:09:05Z","isPatch":true,"sender":{"key":"ruffalolavoisier@gmail.com","avatar":null},"body":"if fseek() is success, return value is 0\n\nSigned-off-by: Ruffalo Lavoisier <RuffaloLavoisier@gmail.com>\n---\n builtin/am.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex d8875ad402..a7727fd4ea 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -589,7 +589,7 @@ static int is_mail(FILE *fp)\n \tregex_t regex;\n \tint ret = 1;\n \n-\tif (fseek(fp, 0L, SEEK_SET))\n+\tif (!fseek(fp, 0L, SEEK_SET))\n \t\tdie_errno(_(\"fseek failed\"));\n \n \tif (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))\n-- \n2.46.1\n\n"},{"id":"503204","messageId":"7601dcef-e256-4e20-bbd4-75955e5a45d5@app.fastmail.com","threadId":"62166","inReplyTo":"20240921150855.31574-1-RuffaloLavoisier@gmail.com","subject":"Re: [PATCH] am: fix condition check on fseek","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-09-21T19:52:50Z","receivedAt":"2024-09-21T19:54:49Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Sat, Sep 21, 2024, at 17:08, Ruffalo Lavoisier wrote:\n> if fseek() is success, return value is 0\n>\n> Signed-off-by: Ruffalo Lavoisier <RuffaloLavoisier@gmail.com>\n> ---\n>  builtin/am.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/am.c b/builtin/am.c\n> index d8875ad402..a7727fd4ea 100644\n> --- a/builtin/am.c\n> +++ b/builtin/am.c\n> @@ -589,7 +589,7 @@ static int is_mail(FILE *fp)\n>  \tregex_t regex;\n>  \tint ret = 1;\n>\n> -\tif (fseek(fp, 0L, SEEK_SET))\n> +\tif (!fseek(fp, 0L, SEEK_SET))\n>  \t\tdie_errno(_(\"fseek failed\"));\n>\n>  \tif (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))\n> --\n> 2.46.1\n\nI don’t get this change? The function returns false on success. true if\nit fails (not zero). You want the program to die if it returns non-zero.\n\nIt’s hard to wrap my head around… “false must mean “no errors” ”\n\nIf the original code has a bug then I don’t see how git-am(1) could work\nconsidering it presumably always checks ‘is_mail’.\n\n-- \nKristoffer\n\n"},{"id":"503266","messageId":"xmqqcyku48yq.fsf@gitster.g","threadId":"62166","inReplyTo":"7601dcef-e256-4e20-bbd4-75955e5a45d5@app.fastmail.com","subject":"Re: [PATCH] am: fix condition check on fseek","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-23T16:43:09Z","receivedAt":"2024-09-23T16:43:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <kristofferhaugsbakk@fastmail.com> writes:\n\n> On Sat, Sep 21, 2024, at 17:08, Ruffalo Lavoisier wrote:\n>> if fseek() is success, return value is 0\n>>\n>> Signed-off-by: Ruffalo Lavoisier <RuffaloLavoisier@gmail.com>\n>> ---\n>>  builtin/am.c | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/am.c b/builtin/am.c\n>> index d8875ad402..a7727fd4ea 100644\n>> --- a/builtin/am.c\n>> +++ b/builtin/am.c\n>> @@ -589,7 +589,7 @@ static int is_mail(FILE *fp)\n>>  \tregex_t regex;\n>>  \tint ret = 1;\n>>\n>> -\tif (fseek(fp, 0L, SEEK_SET))\n>> +\tif (!fseek(fp, 0L, SEEK_SET))\n>>  \t\tdie_errno(_(\"fseek failed\"));\n>>\n>>  \tif (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))\n>> --\n>> 2.46.1\n>\n> I don’t get this change? The function returns false on success. true if\n> it fails (not zero). You want the program to die if it returns non-zero.\n>\n> It’s hard to wrap my head around… “false must mean “no errors” ”\n>\n> If the original code has a bug then I don’t see how git-am(1) could work\n> considering it presumably always checks ‘is_mail’.\n\nYeah, the proposed log message states a correct fact, but it is\nunclear how that justifies the change in the patch part of the\nmessage.\n"}]}