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 prefer therefore to keep it as it is, with a comment about why it is done so.
I prefer we set a good example :)
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
