Is it easier to review this change for correctness if it is three patches or one? TLDR; + struct wpa_key_t *key = &priv->wpa.key[index]; - memcpy(&priv->wpa.key[index].rx_seq[0], enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); + memcpy(key->rx_seq, enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); Brief description of steps: 1. Add local pointer variable, defined to correct memory location. 2. Use newly defined local variable where suitable. 3. Remove unnecessary address operator (reasoning specified below). Extended description of steps: 1. Add local variable (defined in one statement here for brevity) struct wpa_key_t *key = &priv->wpa.key[index]; 2. Use newly defined local variable where suitable. sample original code ... } else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) { memcpy(&priv->wpa.key[index].rx_seq[0], enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); ... sample code after step 2: ... } else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) { memcpy(&key->rx_seq[0], enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); ... Justification for step 3: In the above call to memcpy() taking the address of the initial element of the array adds no extra information to the call since the source variable is named identically and does not take the address of the first element. sample final state: ... } else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) { memcpy(key->rx_seq, enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); ... thanks for taking the time to read. Tobin.
On Thu, 30 Mar 2017 11:22:40 +1100, "Tobin C. Harding" said:
Is it easier to review this change for correctness if it is three patches or one?
TLDR; + struct wpa_key_t *key = &priv->wpa.key[index];
- memcpy(&priv->wpa.key[index].rx_seq[0], enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); + memcpy(key->rx_seq, enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
One patch for one thing.
Brief description of steps: 1. Add local pointer variable, defined to correct memory location. 2. Use newly defined local variable where suitable. 3. Remove unnecessary address operator (reasoning specified below).
So, is this 3 things? Or one thing: "Simplify overly-complex first argument to memcpy()"?
On Wed, Mar 29, 2017 at 08:55:02PM -0400, valdis.kletnieks@vt.edu wrote:
On Thu, 30 Mar 2017 11:22:40 +1100, "Tobin C. Harding" said:
Is it easier to review this change for correctness if it is three patches or one?
TLDR; + struct wpa_key_t *key = &priv->wpa.key[index];
- memcpy(&priv->wpa.key[index].rx_seq[0], enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE); + memcpy(key->rx_seq, enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
One patch for one thing.
Brief description of steps: 1. Add local pointer variable, defined to correct memory location. 2. Use newly defined local variable where suitable. 3. Remove unnecessary address operator (reasoning specified below).
So, is this 3 things? Or one thing:
"Simplify overly-complex first argument to memcpy()"?
Thanks Valdis, very well explained. Tobin.
participants (2)
-
Tobin C. Harding -
valdis.kletnieks@vt.edu