[pbs-devel] [PATCH proxmox-backup] server/worker_task: remove all trailing newlines when reading task status

[pbs-devel] [PATCH proxmox-backup] server/worker_task: remove all trailing newlines when reading task status

[Dominik Csapak]

sometimes we may write multiple newlines at the end of tasks,
for example if an error message contains multiple newlines itself

simply ignore all newlines at the end of task logs instead of only one

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
it happened to me during tape cleaning testing, a permission denied
error had two newlines at the end, and i did not find the source of
them... anyway i think we should ignore all of them

 src/server/worker_task.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 967814c9..a4b2a35f 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -191,7 +191,7 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
     file.read_to_end(&mut data)?;
 
     // task logs should end with newline, we do not want it here
-    if !data.is_empty() && data[data.len()-1] == b'\n' {
+    while !data.is_empty() && data[data.len()-1] == b'\n' {
         data.pop();
     }
 
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup] server/worker_task: remove all trailing newlines when reading task status

[Fabian Grünbichler]

On January 25, 2021 10:29 am, Dominik Csapak wrote:
> sometimes we may write multiple newlines at the end of tasks,
> for example if an error message contains multiple newlines itself
> 
> simply ignore all newlines at the end of task logs instead of only one
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> it happened to me during tape cleaning testing, a permission denied
> error had two newlines at the end, and i did not find the source of
> them... anyway i think we should ignore all of them
> 
>  src/server/worker_task.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
> index 967814c9..a4b2a35f 100644
> --- a/src/server/worker_task.rs
> +++ b/src/server/worker_task.rs
> @@ -191,7 +191,7 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
>      file.read_to_end(&mut data)?;
>  
>      // task logs should end with newline, we do not want it here
> -    if !data.is_empty() && data[data.len()-1] == b'\n' {
> +    while !data.is_empty() && data[data.len()-1] == b'\n' {

while data.last() == Some(&b'\n') {

(same assembly, no potential to introduce off-by-one in the future?)

>          data.pop();
>      }
>  
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 

Re: [pbs-devel] [PATCH proxmox-backup] server/worker_task: remove all trailing newlines when reading task status

[Dominik Csapak]

On 1/25/21 11:37 AM, Fabian Grünbichler wrote:
> On January 25, 2021 10:29 am, Dominik Csapak wrote:
>> sometimes we may write multiple newlines at the end of tasks,
>> for example if an error message contains multiple newlines itself
>>
>> simply ignore all newlines at the end of task logs instead of only one
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> it happened to me during tape cleaning testing, a permission denied
>> error had two newlines at the end, and i did not find the source of
>> them... anyway i think we should ignore all of them
>>
>>   src/server/worker_task.rs | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
>> index 967814c9..a4b2a35f 100644
>> --- a/src/server/worker_task.rs
>> +++ b/src/server/worker_task.rs
>> @@ -191,7 +191,7 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
>>       file.read_to_end(&mut data)?;
>>   
>>       // task logs should end with newline, we do not want it here
>> -    if !data.is_empty() && data[data.len()-1] == b'\n' {
>> +    while !data.is_empty() && data[data.len()-1] == b'\n' {
> 
> while data.last() == Some(&b'\n') {
> 
> (same assembly, no potential to introduce off-by-one in the future?)
> 

yeah sounds right, the change from if to while was too obvious
did not think about how to do it better altogether ;)

>>           data.pop();
>>       }
>>   
>> -- 
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>