From 68ec6dab8c733b92772dace938a4a9eba6ce3494 Mon Sep 17 00:00:00 2001 From: Avi B <474746+abrender@users.noreply.github.com> Date: Thu, 10 Nov 2022 21:13:01 -0500 Subject: Return error from RequestNetbootv4 The current behavior of `RequestNetbootv4` is potentially dangerous because when it reaches the max number of retries, it returns `conversation, nil` (line 91) back to the caller, even if `client.Exchange()` (line 77) returned an `err`. This leads to a situation where the caller will see `err == nil` but incorrect data returned via the other parameter. This changes the behavior to match the behavior of `RequestNetbootv6`. If this change isn't allowed because it's not backwards compatible then we should at least consider returning `nil, nil` instead of `conversation, nil` on the final retry. Signed-off-by: Avi --- netboot/netboot.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/netboot/netboot.go b/netboot/netboot.go index b32f69e..adfc7f3 100644 --- a/netboot/netboot.go +++ b/netboot/netboot.go @@ -48,7 +48,6 @@ func RequestNetbootv6(ifname string, timeout time.Duration, retries int, modifie if err != nil { log.Printf("Client.Exchange failed: %v", err) if i >= retries { - // don't wait at the end of the last attempt return nil, fmt.Errorf("netboot failed after %d attempts: %v", retries+1, err) } log.Printf("sleeping %v before retrying", delay) @@ -80,8 +79,7 @@ func RequestNetbootv4(ifname string, timeout time.Duration, retries int, modifie log.Printf("Client.Exchange failed: %v", err) log.Printf("sleeping %v before retrying", delay) if i >= retries { - // don't wait at the end of the last attempt - break + return nil, fmt.Errorf("netboot failed after %d attempts: %v", retries+1, err) } sleeper(delay) // TODO add random splay -- cgit v1.2.3