Commit c6a1fa12 authored by Johannes Berg's avatar Johannes Berg Committed by John W. Linville

mac80211: minor code cleanups

Nothing very interesting, some checkpatch inspired stuff,
some other things.
Signed-off-by: default avatarJohannes Berg <johannes@sipsolutions.net>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 36ff382d
...@@ -137,7 +137,7 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, ...@@ -137,7 +137,7 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:"); p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:");
for (i = 0; i < STA_TID_NUM; i++) for (i = 0; i < STA_TID_NUM; i++)
p += scnprintf(p, sizeof(buf)+buf-p, "%5d", p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
sta->ampdu_mlme.tid_state_rx[i]? sta->ampdu_mlme.tid_state_rx[i] ?
sta->ampdu_mlme.tid_rx[i]->dialog_token : 0); sta->ampdu_mlme.tid_rx[i]->dialog_token : 0);
p += scnprintf(p, sizeof(buf)+buf-p, "\n TX :"); p += scnprintf(p, sizeof(buf)+buf-p, "\n TX :");
...@@ -148,13 +148,13 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, ...@@ -148,13 +148,13 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:"); p += scnprintf(p, sizeof(buf)+buf-p, "\n DTKN:");
for (i = 0; i < STA_TID_NUM; i++) for (i = 0; i < STA_TID_NUM; i++)
p += scnprintf(p, sizeof(buf)+buf-p, "%5d", p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
sta->ampdu_mlme.tid_state_tx[i]? sta->ampdu_mlme.tid_state_tx[i] ?
sta->ampdu_mlme.tid_tx[i]->dialog_token : 0); sta->ampdu_mlme.tid_tx[i]->dialog_token : 0);
p += scnprintf(p, sizeof(buf)+buf-p, "\n SSN :"); p += scnprintf(p, sizeof(buf)+buf-p, "\n SSN :");
for (i = 0; i < STA_TID_NUM; i++) for (i = 0; i < STA_TID_NUM; i++)
p += scnprintf(p, sizeof(buf)+buf-p, "%5d", p += scnprintf(p, sizeof(buf)+buf-p, "%5d",
sta->ampdu_mlme.tid_state_tx[i]? sta->ampdu_mlme.tid_state_tx[i] ?
sta->ampdu_mlme.tid_tx[i]->ssn : 0); sta->ampdu_mlme.tid_tx[i]->ssn : 0);
p += scnprintf(p, sizeof(buf)+buf-p, "\n"); p += scnprintf(p, sizeof(buf)+buf-p, "\n");
......
...@@ -1013,7 +1013,7 @@ static int __init ieee80211_init(void) ...@@ -1013,7 +1013,7 @@ static int __init ieee80211_init(void)
BUILD_BUG_ON(sizeof(struct ieee80211_tx_info) > sizeof(skb->cb)); BUILD_BUG_ON(sizeof(struct ieee80211_tx_info) > sizeof(skb->cb));
BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, driver_data) + BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, driver_data) +
IEEE80211_TX_INFO_DRIVER_DATA_SIZE > sizeof(skb->cb)); IEEE80211_TX_INFO_DRIVER_DATA_SIZE > sizeof(skb->cb));
ret = rc80211_minstrel_init(); ret = rc80211_minstrel_init();
if (ret) if (ret)
......
...@@ -473,7 +473,7 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata, ...@@ -473,7 +473,7 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
size_t len, size_t len,
struct ieee80211_rx_status *rx_status) struct ieee80211_rx_status *rx_status)
{ {
struct ieee80211_local *local= sdata->local; struct ieee80211_local *local = sdata->local;
struct ieee802_11_elems elems; struct ieee802_11_elems elems;
struct ieee80211_channel *channel; struct ieee80211_channel *channel;
u64 supp_rates = 0; u64 supp_rates = 0;
......
...@@ -49,7 +49,7 @@ ...@@ -49,7 +49,7 @@
/* Arithmetic right shift for positive and negative values for ISO C. */ /* Arithmetic right shift for positive and negative values for ISO C. */
#define RC_PID_DO_ARITH_RIGHT_SHIFT(x, y) \ #define RC_PID_DO_ARITH_RIGHT_SHIFT(x, y) \
(x) < 0 ? -((-(x)) >> (y)) : (x) >> (y) ((x) < 0 ? -((-(x)) >> (y)) : (x) >> (y))
enum rc_pid_event_type { enum rc_pid_event_type {
RC_PID_EVENT_TYPE_TX_STATUS, RC_PID_EVENT_TYPE_TX_STATUS,
......
...@@ -26,10 +26,11 @@ ...@@ -26,10 +26,11 @@
#include "tkip.h" #include "tkip.h"
#include "wme.h" #include "wme.h"
u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw, static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx, struct tid_ampdu_rx *tid_agg_rx,
struct sk_buff *skb, u16 mpdu_seq_num, struct sk_buff *skb,
int bar_req); u16 mpdu_seq_num,
int bar_req);
/* /*
* monitor mode reception * monitor mode reception
* *
...@@ -1988,17 +1989,17 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw, ...@@ -1988,17 +1989,17 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
static inline int seq_less(u16 sq1, u16 sq2) static inline int seq_less(u16 sq1, u16 sq2)
{ {
return (((sq1 - sq2) & SEQ_MASK) > (SEQ_MODULO >> 1)); return ((sq1 - sq2) & SEQ_MASK) > (SEQ_MODULO >> 1);
} }
static inline u16 seq_inc(u16 sq) static inline u16 seq_inc(u16 sq)
{ {
return ((sq + 1) & SEQ_MASK); return (sq + 1) & SEQ_MASK;
} }
static inline u16 seq_sub(u16 sq1, u16 sq2) static inline u16 seq_sub(u16 sq1, u16 sq2)
{ {
return ((sq1 - sq2) & SEQ_MASK); return (sq1 - sq2) & SEQ_MASK;
} }
...@@ -2006,10 +2007,11 @@ static inline u16 seq_sub(u16 sq1, u16 sq2) ...@@ -2006,10 +2007,11 @@ static inline u16 seq_sub(u16 sq1, u16 sq2)
* As it function blongs to Rx path it must be called with * As it function blongs to Rx path it must be called with
* the proper rcu_read_lock protection for its flow. * the proper rcu_read_lock protection for its flow.
*/ */
u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw, static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx, struct tid_ampdu_rx *tid_agg_rx,
struct sk_buff *skb, u16 mpdu_seq_num, struct sk_buff *skb,
int bar_req) u16 mpdu_seq_num,
int bar_req)
{ {
struct ieee80211_local *local = hw_to_local(hw); struct ieee80211_local *local = hw_to_local(hw);
struct ieee80211_rx_status status; struct ieee80211_rx_status status;
......
...@@ -290,7 +290,7 @@ int sta_info_insert(struct sta_info *sta) ...@@ -290,7 +290,7 @@ int sta_info_insert(struct sta_info *sta)
} }
if (WARN_ON(compare_ether_addr(sta->sta.addr, sdata->dev->dev_addr) == 0 || if (WARN_ON(compare_ether_addr(sta->sta.addr, sdata->dev->dev_addr) == 0 ||
is_multicast_ether_addr(sta->sta.addr))) { is_multicast_ether_addr(sta->sta.addr))) {
err = -EINVAL; err = -EINVAL;
goto out_free; goto out_free;
} }
...@@ -821,7 +821,7 @@ void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata, ...@@ -821,7 +821,7 @@ void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata,
} }
struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw, struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
const u8 *addr) const u8 *addr)
{ {
struct sta_info *sta = sta_info_get(hw_to_local(hw), addr); struct sta_info *sta = sta_info_get(hw_to_local(hw), addr);
......
...@@ -49,17 +49,19 @@ void ieee80211_wep_free(struct ieee80211_local *local) ...@@ -49,17 +49,19 @@ void ieee80211_wep_free(struct ieee80211_local *local)
crypto_free_blkcipher(local->wep_rx_tfm); crypto_free_blkcipher(local->wep_rx_tfm);
} }
static inline int ieee80211_wep_weak_iv(u32 iv, int keylen) static inline bool ieee80211_wep_weak_iv(u32 iv, int keylen)
{ {
/* Fluhrer, Mantin, and Shamir have reported weaknesses in the /*
* Fluhrer, Mantin, and Shamir have reported weaknesses in the
* key scheduling algorithm of RC4. At least IVs (KeyByte + 3, * key scheduling algorithm of RC4. At least IVs (KeyByte + 3,
* 0xff, N) can be used to speedup attacks, so avoid using them. */ * 0xff, N) can be used to speedup attacks, so avoid using them.
*/
if ((iv & 0xff00) == 0xff00) { if ((iv & 0xff00) == 0xff00) {
u8 B = (iv >> 16) & 0xff; u8 B = (iv >> 16) & 0xff;
if (B >= 3 && B < 3 + keylen) if (B >= 3 && B < 3 + keylen)
return 1; return true;
} }
return 0; return false;
} }
...@@ -268,7 +270,7 @@ int ieee80211_wep_decrypt(struct ieee80211_local *local, struct sk_buff *skb, ...@@ -268,7 +270,7 @@ int ieee80211_wep_decrypt(struct ieee80211_local *local, struct sk_buff *skb,
} }
u8 * ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key) bool ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key)
{ {
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
unsigned int hdrlen; unsigned int hdrlen;
...@@ -276,16 +278,13 @@ u8 * ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key) ...@@ -276,16 +278,13 @@ u8 * ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key)
u32 iv; u32 iv;
if (!ieee80211_has_protected(hdr->frame_control)) if (!ieee80211_has_protected(hdr->frame_control))
return NULL; return false;
hdrlen = ieee80211_hdrlen(hdr->frame_control); hdrlen = ieee80211_hdrlen(hdr->frame_control);
ivpos = skb->data + hdrlen; ivpos = skb->data + hdrlen;
iv = (ivpos[0] << 16) | (ivpos[1] << 8) | ivpos[2]; iv = (ivpos[0] << 16) | (ivpos[1] << 8) | ivpos[2];
if (ieee80211_wep_weak_iv(iv, key->conf.keylen)) return ieee80211_wep_weak_iv(iv, key->conf.keylen);
return ivpos;
return NULL;
} }
ieee80211_rx_result ieee80211_rx_result
...@@ -329,6 +328,8 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb) ...@@ -329,6 +328,8 @@ static int wep_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
ieee80211_tx_result ieee80211_tx_result
ieee80211_crypto_wep_encrypt(struct ieee80211_tx_data *tx) ieee80211_crypto_wep_encrypt(struct ieee80211_tx_data *tx)
{ {
int i;
ieee80211_tx_set_protected(tx); ieee80211_tx_set_protected(tx);
if (wep_encrypt_skb(tx, tx->skb) < 0) { if (wep_encrypt_skb(tx, tx->skb) < 0) {
...@@ -337,9 +338,8 @@ ieee80211_crypto_wep_encrypt(struct ieee80211_tx_data *tx) ...@@ -337,9 +338,8 @@ ieee80211_crypto_wep_encrypt(struct ieee80211_tx_data *tx)
} }
if (tx->extra_frag) { if (tx->extra_frag) {
int i;
for (i = 0; i < tx->num_extra_frag; i++) { for (i = 0; i < tx->num_extra_frag; i++) {
if (wep_encrypt_skb(tx, tx->extra_frag[i]) < 0) { if (wep_encrypt_skb(tx, tx->extra_frag[i])) {
I802_DEBUG_INC(tx->local-> I802_DEBUG_INC(tx->local->
tx_handlers_drop_wep); tx_handlers_drop_wep);
return TX_DROP; return TX_DROP;
......
...@@ -26,7 +26,7 @@ int ieee80211_wep_encrypt(struct ieee80211_local *local, struct sk_buff *skb, ...@@ -26,7 +26,7 @@ int ieee80211_wep_encrypt(struct ieee80211_local *local, struct sk_buff *skb,
struct ieee80211_key *key); struct ieee80211_key *key);
int ieee80211_wep_decrypt(struct ieee80211_local *local, struct sk_buff *skb, int ieee80211_wep_decrypt(struct ieee80211_local *local, struct sk_buff *skb,
struct ieee80211_key *key); struct ieee80211_key *key);
u8 *ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key); bool ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key);
ieee80211_rx_result ieee80211_rx_result
ieee80211_crypto_wep_decrypt(struct ieee80211_rx_data *rx); ieee80211_crypto_wep_decrypt(struct ieee80211_rx_data *rx);
......
...@@ -49,8 +49,7 @@ ieee80211_tx_h_michael_mic_add(struct ieee80211_tx_data *tx) ...@@ -49,8 +49,7 @@ ieee80211_tx_h_michael_mic_add(struct ieee80211_tx_data *tx)
!(tx->flags & IEEE80211_TX_FRAGMENTED) && !(tx->flags & IEEE80211_TX_FRAGMENTED) &&
!(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC) && !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC) &&
!wpa_test) { !wpa_test) {
/* hwaccel - with no need for preallocated room for Michael MIC /* hwaccel - with no need for preallocated room for MMIC */
*/
return TX_CONTINUE; return TX_CONTINUE;
} }
...@@ -67,8 +66,6 @@ ieee80211_tx_h_michael_mic_add(struct ieee80211_tx_data *tx) ...@@ -67,8 +66,6 @@ ieee80211_tx_h_michael_mic_add(struct ieee80211_tx_data *tx)
#else #else
authenticator = 1; authenticator = 1;
#endif #endif
/* At this point we know we're using ALG_TKIP. To get the MIC key
* we now will rely on the offset from the ieee80211_key_conf::key */
key_offset = authenticator ? key_offset = authenticator ?
NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY : NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY :
NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY; NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY;
...@@ -91,9 +88,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx) ...@@ -91,9 +88,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
struct sk_buff *skb = rx->skb; struct sk_buff *skb = rx->skb;
int authenticator = 1, wpa_test = 0; int authenticator = 1, wpa_test = 0;
/* /* No way to verify the MIC if the hardware stripped it */
* No way to verify the MIC if the hardware stripped it
*/
if (rx->status->flag & RX_FLAG_MMIC_STRIPPED) if (rx->status->flag & RX_FLAG_MMIC_STRIPPED)
return RX_CONTINUE; return RX_CONTINUE;
...@@ -115,8 +110,6 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx) ...@@ -115,8 +110,6 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
#else #else
authenticator = 1; authenticator = 1;
#endif #endif
/* At this point we know we're using ALG_TKIP. To get the MIC key
* we now will rely on the offset from the ieee80211_key_conf::key */
key_offset = authenticator ? key_offset = authenticator ?
NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY : NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY :
NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY; NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY;
...@@ -201,6 +194,7 @@ ieee80211_tx_result ...@@ -201,6 +194,7 @@ ieee80211_tx_result
ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx) ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
{ {
struct sk_buff *skb = tx->skb; struct sk_buff *skb = tx->skb;
int i;
ieee80211_tx_set_protected(tx); ieee80211_tx_set_protected(tx);
...@@ -208,9 +202,8 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx) ...@@ -208,9 +202,8 @@ ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
return TX_DROP; return TX_DROP;
if (tx->extra_frag) { if (tx->extra_frag) {
int i;
for (i = 0; i < tx->num_extra_frag; i++) { for (i = 0; i < tx->num_extra_frag; i++) {
if (tkip_encrypt_skb(tx, tx->extra_frag[i]) < 0) if (tkip_encrypt_skb(tx, tx->extra_frag[i]))
return TX_DROP; return TX_DROP;
} }
} }
...@@ -348,7 +341,7 @@ static inline void ccmp_pn2hdr(u8 *hdr, u8 *pn, int key_id) ...@@ -348,7 +341,7 @@ static inline void ccmp_pn2hdr(u8 *hdr, u8 *pn, int key_id)
} }
static inline int ccmp_hdr2pn(u8 *pn, u8 *hdr) static inline void ccmp_hdr2pn(u8 *pn, u8 *hdr)
{ {
pn[0] = hdr[7]; pn[0] = hdr[7];
pn[1] = hdr[6]; pn[1] = hdr[6];
...@@ -356,7 +349,6 @@ static inline int ccmp_hdr2pn(u8 *pn, u8 *hdr) ...@@ -356,7 +349,6 @@ static inline int ccmp_hdr2pn(u8 *pn, u8 *hdr)
pn[3] = hdr[4]; pn[3] = hdr[4];
pn[4] = hdr[1]; pn[4] = hdr[1];
pn[5] = hdr[0]; pn[5] = hdr[0];
return (hdr[3] >> 6) & 0x03;
} }
...@@ -371,7 +363,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb) ...@@ -371,7 +363,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
if ((tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) && if ((tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) &&
!(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) { !(tx->key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
/* hwaccel - with no need for preallocated room for CCMP " /* hwaccel - with no need for preallocated room for CCMP
* header or MIC fields */ * header or MIC fields */
info->control.hw_key = &tx->key->conf; info->control.hw_key = &tx->key->conf;
return 0; return 0;
...@@ -424,6 +416,7 @@ ieee80211_tx_result ...@@ -424,6 +416,7 @@ ieee80211_tx_result
ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx) ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
{ {
struct sk_buff *skb = tx->skb; struct sk_buff *skb = tx->skb;
int i;
ieee80211_tx_set_protected(tx); ieee80211_tx_set_protected(tx);
...@@ -431,9 +424,8 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx) ...@@ -431,9 +424,8 @@ ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
return TX_DROP; return TX_DROP;
if (tx->extra_frag) { if (tx->extra_frag) {
int i;
for (i = 0; i < tx->num_extra_frag; i++) { for (i = 0; i < tx->num_extra_frag; i++) {
if (ccmp_encrypt_skb(tx, tx->extra_frag[i]) < 0) if (ccmp_encrypt_skb(tx, tx->extra_frag[i]))
return TX_DROP; return TX_DROP;
} }
} }
...@@ -465,7 +457,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx) ...@@ -465,7 +457,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx)
(rx->status->flag & RX_FLAG_IV_STRIPPED)) (rx->status->flag & RX_FLAG_IV_STRIPPED))
return RX_CONTINUE; return RX_CONTINUE;
(void) ccmp_hdr2pn(pn, skb->data + hdrlen); ccmp_hdr2pn(pn, skb->data + hdrlen);
if (memcmp(pn, key->u.ccmp.rx_pn[rx->queue], CCMP_PN_LEN) <= 0) { if (memcmp(pn, key->u.ccmp.rx_pn[rx->queue], CCMP_PN_LEN) <= 0) {
key->u.ccmp.replays++; key->u.ccmp.replays++;
...@@ -480,9 +472,8 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx) ...@@ -480,9 +472,8 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx)
key->u.ccmp.tfm, key->u.ccmp.rx_crypto_buf, key->u.ccmp.tfm, key->u.ccmp.rx_crypto_buf,
skb->data + hdrlen + CCMP_HDR_LEN, data_len, skb->data + hdrlen + CCMP_HDR_LEN, data_len,
skb->data + skb->len - CCMP_MIC_LEN, skb->data + skb->len - CCMP_MIC_LEN,
skb->data + hdrlen + CCMP_HDR_LEN)) { skb->data + hdrlen + CCMP_HDR_LEN))
return RX_DROP_UNUSABLE; return RX_DROP_UNUSABLE;
}
} }
memcpy(key->u.ccmp.rx_pn[rx->queue], pn, CCMP_PN_LEN); memcpy(key->u.ccmp.rx_pn[rx->queue], pn, CCMP_PN_LEN);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment