Re: [PATCH] sequencer.c: unify error messages

[Ralf Thielow]

2017-10-13 23:12 GMT+02:00 René Scharfe :
> Am 13.10.2017 um 19:51 schrieb Ralf Thielow:
>> When ftruncate() in rearrange_squash() fails, we write
>> that we couldn't finish the operation on the todo file.
>> It is more accurate to write that we couldn't truncate
>> as we do in other calls of ftruncate().
>
> Would it make sense to factor out rewriting the to-do file to avoid
> code duplication in the first place?
>
>> While at there, remove a full stop in another error message.
>>
>> Signed-off-by: Ralf Thielow 
>> ---
>>   sequencer.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index e258bb646..b0e6459a5 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>>   if (fd < 0)
>>   res = error_errno(_("could not open '%s'"), todo_file);
>>   else if (write(fd, buf.buf, buf.len) < 0)
>> - res = error_errno(_("could not read '%s'."), 
>> todo_file);
>> + res = error_errno(_("could not read '%s'"), todo_file);
>
> That should read "write", right?
>

Sure. I'll send a new version of this patch to fix the messages. Maybe
someone else picks up the other things. Thanks.

>>   else if (ftruncate(fd, buf.len) < 0)
>> - res = error_errno(_("could not finish '%s'"),
>> + res = error_errno(_("could not truncate '%s'"),
>>  todo_file);
>
> Hmm, why call ftruncate(2) instead of opening the file with O_TRUNC?
>
>>   close(fd);
>>   strbuf_release();
>>

Re: [PATCH] sequencer.c: unify error messages

[Johannes Schindelin]

Hi Ralf,

On Fri, 13 Oct 2017, Ralf Thielow wrote:

> When ftruncate() in rearrange_squash() fails, we write
> that we couldn't finish the operation on the todo file.
> It is more accurate to write that we couldn't truncate
> as we do in other calls of ftruncate().
> 
> While at there, remove a full stop in another error message.

Thanks, looks good (maybe s/read/write/ as pointed out by René, though?).

Ciao,
Dscho

Re: [PATCH] sequencer.c: unify error messages

[René Scharfe]

Am 13.10.2017 um 19:51 schrieb Ralf Thielow:
> When ftruncate() in rearrange_squash() fails, we write
> that we couldn't finish the operation on the todo file.
> It is more accurate to write that we couldn't truncate
> as we do in other calls of ftruncate().

Would it make sense to factor out rewriting the to-do file to avoid
code duplication in the first place?

> While at there, remove a full stop in another error message.
> 
> Signed-off-by: Ralf Thielow 
> ---
>   sequencer.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index e258bb646..b0e6459a5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2948,9 +2948,9 @@ int rearrange_squash(void)
>   if (fd < 0)
>   res = error_errno(_("could not open '%s'"), todo_file);
>   else if (write(fd, buf.buf, buf.len) < 0)
> - res = error_errno(_("could not read '%s'."), todo_file);
> + res = error_errno(_("could not read '%s'"), todo_file);
   
That should read "write", right?

>   else if (ftruncate(fd, buf.len) < 0)
> - res = error_errno(_("could not finish '%s'"),
> + res = error_errno(_("could not truncate '%s'"),
>  todo_file);

Hmm, why call ftruncate(2) instead of opening the file with O_TRUNC?

>   close(fd);
>   strbuf_release();
> 

[PATCH] sequencer.c: unify error messages

[Ralf Thielow]

When ftruncate() in rearrange_squash() fails, we write
that we couldn't finish the operation on the todo file.
It is more accurate to write that we couldn't truncate
as we do in other calls of ftruncate().

While at there, remove a full stop in another error message.

Signed-off-by: Ralf Thielow 
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index e258bb646..b0e6459a5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,9 +2948,9 @@ int rearrange_squash(void)
if (fd < 0)
res = error_errno(_("could not open '%s'"), todo_file);
else if (write(fd, buf.buf, buf.len) < 0)
-   res = error_errno(_("could not read '%s'."), todo_file);
+   res = error_errno(_("could not read '%s'"), todo_file);
else if (ftruncate(fd, buf.len) < 0)
-   res = error_errno(_("could not finish '%s'"),
+   res = error_errno(_("could not truncate '%s'"),
   todo_file);
close(fd);
strbuf_release();
-- 
2.15.0.rc0.296.g7b26d72