Hi Jouni, First of all, thanks for the review, I submitted a v2 patch list for the comments changes. However, for the enum values how can we get acknowledgement from existing users of the interface? should I submit another patch list for these changes and keep it under review to get other users feedback? Thanks On Tue, Jan 11, 2022 at 2:49 PM Jouni Malinen <j@xxxxx> wrote: > > On Tue, Jan 11, 2022 at 11:10:09AM -0800, Arowa Suliman wrote: > > Replace the word "whitelist" with the inclusive word "allowlist" > > > diff --git a/src/common/qca-vendor.h b/src/common/qca-vendor.h > > > - /* Attributes for wifi_set_ssid_white_list */ > > - QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_WHITE_LIST_SSID_NUM_NETWORKS = 3, > > - QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_WHITE_LIST_SSID_LIST = 4, > > - QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_WHITE_LIST_SSID = 5, > > + /* Attributes for wifi_set_ssid_allow_list */ > > + QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_ALLOW_LIST_SSID_NUM_NETWORKS = 3, > > + QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_ALLOW_LIST_SSID_LIST = 4, > > + QCA_WLAN_VENDOR_ATTR_ROAMING_PARAM_ALLOW_LIST_SSID = 5, > > It would be fine to modify the comments in this file, but the enum > values specified here defines a kernel interface that is used by > external components (i.e., drivers implementing this interface and user > space components using the interface). While hostap.git is the > authoritative location for the definitions, changing the names here > could break building of external components that copy this file as-is. > As such, I'm a bit hesitant on applying this type of changes at least > before getting some acknowledgment from existing users of the interface. > > The same comment applies for patch 2/2. > > > diff --git a/wpa_supplicant/ap.c b/wpa_supplicant/ap.c > > @@ -382,7 +382,7 @@ int wpa_supplicant_conf_ap_ht(struct wpa_supplicant *wpa_s, > > /* > > - * white-list capabilities that won't cause issues > > + * allow-list capabilities that won't cause issues > > * to connecting stations, while leaving the current > > * capabilities intact (currently disabled SMPS). > > */ > > It would be easier to have this in a separate patch, but as far as the > proposed change is concerned, I'm not sure the simple renaming here is > the best approach. I'd rather reword that to be more readable by stating > something like "include capabilities..". At least for me, both of these > "*-list" as a verb constructions are not exactly clear. > > -- > Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
