Hi Denis, On Fri, Oct 5, 2018 at 4:20 AM Denis Kenzior <[email protected]> wrote: > > Hi Giacinto, > > >> Ugh. I really hate GCC people sometimes. While the warning is strictly > >> correct, it is useless for 99% of the code and forces us to jump through > >> hoops. Anyway, I would propose we use the following pattern: > >> > >> int auth_method_to_qmi(enum ofono_auth_method method, uint8_t *out_auth) > >> { > >> switch (method) > >> case CHAP: > >> *out_auth = ... > >> return 0; > >> case: > >> ... > >> } > >> > >> return -ERANGE; > >> } > >> > > > > I tried but it is worst. Now gcc complains about potentially > > uninitialized variable. > > > > Here is the code > > static int auth_method_to_qmi(enum ofono_gprs_auth_method method, > > uint8_t *out_auth) > > { > > switch (method) { > > case OFONO_GPRS_AUTH_METHOD_CHAP: > > *out_auth = QMI_WDS_AUTHENTICATION_CHAP; > > return 0; > > case OFONO_GPRS_AUTH_METHOD_PAP: > > *out_auth = QMI_WDS_AUTHENTICATION_PAP; > > return 0; > > case OFONO_GPRS_AUTH_METHOD_NONE: > > *out_auth = QMI_WDS_AUTHENTICATION_NONE; > > return 0; > > } > > return -1; > > } > > > > static void qmi_activate_primary(struct ofono_gprs_context *gc, > > [...] > > uint8_t auth; > > [...] > > auth_method_to_qmi(ctx->auth_method, &auth); > > Well you might want to check for an error return here. e.g. > > if (auth_method_to_qmi() < 0) { > CALLBACK_WITH_FAILURE(); > return; > } > > > > > and here the compiler answer: > > > > drivers/qmimodem/gprs-context.c:288:2: error: ‘auth’ may be used > > uninitialized in this function [-Werror=maybe-uninitialized] > > qmi_param_append_uint8(param, QMI_WDS_PARAM_AUTHENTICATION_PREFERENCE, > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > auth); > > ~~~~~ > > > > It doesn't look like we have a clean way out of this. I can > > pre-initialize the variable, add a default to the switch, return the > > value for uint8 out_auth, > > but none of these will give you a clean solution. > > No, we should not initialize variables unnecessarily. This is even > documented: doc/coding-style.txt, item M7
I know this, that's why there are 3 lines of comment on it. > > > > > I prefer therefore to keep it as it is, with a comment about why it is done > > so. > > > > I prefer we set a good example :) Then the cleanest way is to set a default in the switch, without any function. Ok? > > Regards, > -Denis Regards, Giacinto _______________________________________________ ofono mailing list [email protected] https://lists.ofono.org/mailman/listinfo/ofono
