Hi Yang,

On 07/21/2010 06:07 AM, Yang Gu wrote:
> ---
>  src/sim.c     |   77 +++++++++++++++++++++++++++++++++++++++++++-
>  src/simutil.h |   98 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 173 insertions(+), 2 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index 2514e7b..2e304dd 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -95,6 +95,8 @@ struct ofono_sim {
>       void *driver_data;
>       struct ofono_atom *atom;
>       DBusMessage *pending;
> +     gboolean service_alloc[88];
> +     gboolean service_actv[3];

I really think this is overkill.  A gboolean takes up 4 bytes of space,
so this gets a bit expensive fast.  Can't you just store the bitmap and
provide a simple utility function to get the enabled flag?

>  };
>  
>  struct msisdn_set_request {
> @@ -1059,6 +1061,77 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
>       sim->driver->read_imsi(sim, sim_imsi_cb, sim);
>  }
>  
> +static void sim_efest_read_cb(int ok, int length, int record,
> +                             const unsigned char *data,
> +                             int record_length, void *userdata)
> +{
> +     struct ofono_sim *sim = userdata;
> +     int i;
> +     int size;
> +
> +     if (!ok)
> +             return;
> +
> +     if (length < 1) {
> +             ofono_error("EFest shall contain at least one byte");
> +             return;
> +     }
> +
> +     size = sizeof(sim->service_actv) / sizeof(*sim->service_actv);
> +     for (i = 0; i < size; i++) {
> +             if (i / 8 >= length)
> +                     sim->service_actv[i] = 0;
> +             else
> +                     sim->service_actv[i] = (data[i / 8] >> (i % 8)) & 1;
> +     }
> +}
> +
> +static void sim_retrieve_efest(struct ofono_sim *sim)
> +{
> +     ofono_sim_read(sim, SIM_EFEST_FILEID,
> +                     OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +                     sim_efest_read_cb, sim);
> +}

What is the purpose of the above function?  Using it inline might be better.

> +
> +static void sim_efust_read_cb(int ok, int length, int record,
> +                             const unsigned char *data,
> +                             int record_length, void *userdata)
> +{
> +     struct ofono_sim *sim = userdata;
> +     int i;
> +     int size;
> +
> +     if (!ok)
> +             return;
> +
> +     if (length < 1) {
> +             ofono_error("EFust shall contain at least one byte");
> +             return;
> +     }
> +
> +     size = sizeof(sim->service_alloc) / sizeof(*sim->service_alloc);
> +     for (i = 0; i < size; i++) {
> +             if (i / 8 >= length)
> +                     sim->service_alloc[i] = 0;
> +             else
> +                     sim->service_alloc[i] = (data[i / 8] >> (i % 8)) & 1;
> +     }
> +}
> +
> +static void sim_retrieve_efust(struct ofono_sim *sim)
> +{
> +     ofono_sim_read(sim, SIM_EFUST_FILEID,
> +                     OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +                     sim_efust_read_cb, sim);
> +}
> +

Again, what is the purpose here?  Moving this down into
sim_service_request might be a good idea.

> +static void sim_service_request(struct ofono_sim *sim)
> +{
> +     sim_retrieve_efust(sim);
> +     sim_retrieve_efest(sim);
> +     sim_retrieve_imsi(sim);

You have to be careful here, the sim_op queue is running in parallel
with sim_retrieve_imsi.  So if your intention is to delay retrieval of
the IMSI until EFust and EFest are read, then you need to retrieve the
IMSI from the EFest callback.

> +}
> +
>  static void sim_pin_query_cb(const struct ofono_error *error,
>                               enum ofono_sim_password_type pin_type,
>                               void *data)
> @@ -1088,13 +1161,13 @@ static void sim_pin_query_cb(const struct ofono_error 
> *error,
>  
>  checkdone:
>       if (pin_type == OFONO_SIM_PASSWORD_NONE)
> -             sim_retrieve_imsi(sim);
> +             sim_service_request(sim);
>  }
>  
>  static void sim_pin_check(struct ofono_sim *sim)
>  {
>       if (!sim->driver->query_passwd_state) {
> -             sim_retrieve_imsi(sim);
> +             sim_service_request(sim);
>               return;
>       }
>  
> diff --git a/src/simutil.h b/src/simutil.h
> index 29194ca..76ac433 100644
> --- a/src/simutil.h
> +++ b/src/simutil.h
> @@ -26,9 +26,11 @@ enum sim_fileid {
>       SIM_EF_CPHS_MWIS_FILEID = 0x6f11,
>       SIM_EF_CPHS_INFORMATION_FILEID = 0x6f16,
>       SIM_EF_CPHS_MBDN_FILEID = 0x6f17,
> +     SIM_EFUST_FILEID = 0x6f38,
>       SIM_EFMSISDN_FILEID = 0x6f40,
>       SIM_EFSPN_FILEID = 0x6f46,
>       SIM_EFSDN_FILEID = 0x6f49,
> +     SIM_EFEST_FILEID = 0x6f56,
>       SIM_EFAD_FILEID = 0x6fad,
>       SIM_EFPHASE_FILEID = 0x6fae,
>       SIM_EFPNN_FILEID = 0x6fc5,
> @@ -53,6 +55,102 @@ enum sim_file_access {
>       SIM_FILE_ACCESS_NEVER = 15,
>  };
>  
> +/* 131.102 Section 4.2.8 */
> +enum usim_alloc_service {
> +     USIM_ALLOC_SERVICE_LOCAL_PHONE_BOOK             = 0,
> +     USIM_ALLOC_SERVICE_FDN                          = 1,
> +     USIM_ALLOC_SERVICE_EXT_2                        = 2,
> +     USIM_ALLOC_SERVICE_SDN                          = 3,
> +     USIM_ALLOC_SERVICE_EXT_3                        = 4,
> +     USIM_ALLOC_SERVICE_BDN                          = 5,
> +     USIM_ALLOC_SERVICE_EXT_4                        = 6,
> +     USIM_ALLOC_SERVICE_OCI_OCT                      = 7,
> +     USIM_ALLOC_SERVICE_ICI_ICT                      = 8,
> +     USIM_ALLOC_SERVICE_SMS                          = 9,
> +     USIM_ALLOC_SERVICE_SMSR                         = 10,
> +     USIM_ALLOC_SERVICE_SMSP                         = 11,
> +     USIM_ALLOC_SERVICE_AOC                          = 12,
> +     USIM_ALLOC_SERVICE_CCP2                         = 13,
> +     USIM_ALLOC_SERVICE_CBS_ID                       = 14,
> +     USIM_ALLOC_SERVICE_CBS_ID_RANGE                 = 15,
> +     USIM_ALLOC_SERVICE_GROUP_ID_LEVEL_1             = 16,
> +     USIM_ALLOC_SERVICE_GROUP_ID_LEVEL_2             = 17,
> +     USIM_ALLOC_SERVICE_PROVIDER_NAME                = 18,
> +     USIM_ALLOC_SERVICE_USER_PLMN                    = 19,
> +     USIM_ALLOC_SERVICE_MSISDN                       = 20,
> +     USIM_ALLOC_SERVICE_IMG                          = 21,
> +     USIM_ALLOC_SERVICE_SOLSA                        = 22,
> +     USIM_ALLOC_SERVICE_PRECEDENCE_PREEMPTION        = 23,
> +     USIM_ALLOC_SERVICE_EMLPP                        = 24,
> +     USIM_ALLOC_SERVICE_GSM_ACCESS                   = 26,
> +     USIM_ALLOC_SERVICE_DATA_DOWNLOAD_SMS_PP         = 27,
> +     USIM_ALLOC_SERVICE_DATA_DOWNLOAD_SMS_CB         = 28,
> +     USIM_ALLOC_SERVICE_CALL_CONTROL_USIM            = 29,
> +     USIM_ALLOC_SERVICE_MO_SMS_USIM                  = 30,
> +     USIM_ALLOC_SERVICE_RUN_AT_COMMAND               = 31,
> +     USIM_ALLOC_SERVICE_ENABLED_SERVICE_TABLE        = 33,
> +     USIM_ALLOC_SERVICE_ACL                          = 34,
> +     USIM_ALLOC_SERVICE_DEPERSONALISATION_CTRL_KEY   = 35,
> +     USIM_ALLOC_SERVICE_NETWORK_LIST                 = 36,
> +     USIM_ALLOC_SERVICE_GSM_SECURITY_CONTEXT         = 37,
> +     USIM_ALLOC_SERVICE_CPBCCH                       = 38,
> +     USIM_ALLOC_SERVICE_INVESTIGATION_SCAN           = 39,
> +     USIM_ALLOC_SERVICE_MEXE                         = 40,
> +     USIM_ALLOC_SERVICE_OPERATOR_PLMN                = 41,
> +     USIM_ALLOC_SERVICE_HPLMN                        = 42,
> +     USIM_ALLOC_SERVICE_EXT_5                        = 43,
> +     USIM_ALLOC_SERVICE_PLMN_NETWORK_NAME            = 44,
> +     USIM_ALLOC_SERVICE_OPERATOR_PLMN_LIST           = 45,
> +     USIM_ALLOC_SERVICE_MAILBOX_DIALLING_NUMBERS     = 46,
> +     USIM_ALLOC_SERVICE_MWIS                         = 47,
> +     USIM_ALLOC_SERVICE_CFIS                         = 48,
> +     USIM_ALLOC_SERVICE_PROVIDER_DISPLAY_INFO        = 50,
> +     USIM_ALLOC_SERVICE_MMS                          = 51,
> +     USIM_ALLOC_SERVICE_EXT_8                        = 52,
> +     USIM_ALLOC_SERVICE_CALL_CONTROL_GPRS_USIM       = 53,
> +     USIM_ALLOC_SERVICE_MMS_USER_CONN_PARAM          = 54,
> +     USIM_ALLOC_SERVICE_NIA                          = 55,
> +     USIM_ALLOC_SERVICE_EFVGCS_EFVGCSS               = 56,
> +     USIM_ALLOC_SERVICE_EFVBS_EFVBSS                 = 57,
> +     USIM_ALLOC_SERVICE_PSEUDONYM                    = 58,
> +     USIM_ALLOC_SERVICE_USER_PLMN_I_WLAN             = 59,
> +     USIM_ALLOC_SERVICE_OPERATOR_PLMN_I_WLAN         = 60,
> +     USIM_ALLOC_SERVICE_USER_WSID                    = 61,
> +     USIM_ALLOC_SERVICE_OPERATOR_WSID                = 62,
> +     USIM_ALLOC_SERVICE_VGCS_SECURITY                = 63,
> +     USIM_ALLOC_SERVICE_VBS_SECURITY                 = 64,
> +     USIM_ALLOC_SERVICE_WLAN_REAUTH_ID               = 65,
> +     USIM_ALLOC_SERVICE_MMS_STORAGE                  = 66,
> +     USIM_ALLOC_SERVICE_GBA                          = 67,
> +     USIM_ALLOC_SERVICE_MBMS_SECURITY                = 68,
> +     USIM_ALLOC_SERVICE_USSD_APPLICATION_MODE        = 69,
> +     USIM_ALLOC_SERVICE_EQUIVALENT_HPLMN             = 70,
> +     USIM_ALLOC_SERVICE_ADDITIONAL_TERMINAL_PROFILE  = 71,
> +     USIM_ALLOC_SERVICE_EQUIVALENT_HPLMN_IND         = 72,
> +     USIM_ALLOC_SERVICE_LAST_RPLMN_IND               = 73,
> +     USIM_ALLOC_SERVICE_OMA_BCAST_SC_PROFILE         = 74,
> +     USIM_ALLOC_SERVICE_BGA_LOCAL_KEY                = 75,
> +     USIM_ALLOC_SERVICE_TERMINAL_APPLICATIONS        = 76,
> +     USIM_ALLOC_SERVICE_PROVIDER_NAME_ICON           = 77,
> +     USIM_ALLOC_SERVICE_PLMN_NETWORK_NAME_ICON       = 78,
> +     USIM_ALLOC_SERVICE_CONN_PARAM_USIM_IP           = 79,
> +     USIM_ALLOC_SERVICE_HOME_I_WLAN_ID_LIST          = 80,
> +     USIM_ALLOC_SERVICE_I_WLAN_EQUIVALENT_HPLMN_IND  = 81,
> +     USIM_ALLOC_SERVICE_I_WLAN_HPLMN_PRIORITY_IND    = 82,
> +     USIM_ALLOC_SERVICE_I_WLAN_LAST_PLMN             = 83,
> +     USIM_ALLOC_SERVICE_EPS_INFO                     = 84,
> +     USIM_ALLOC_SERVICE_CSG_IND                      = 85,
> +     USIM_ALLOC_SERVICE_CALL_CONTROL_EPS_PDN_USIM    = 86,
> +     USIM_ALLOC_SERVICE_HPLMN_DIRECT_ACCESS          = 87
> +};
> +

So I realize what you're trying to do here, but I think that prefixing
this enum with SIM_UST_SERVICE will be better.  Also, since you're at
it, make sure you grab the last couple of enums from 31.102.

> +/* 131.102 Section 4.2.47 */
> +enum usim_actv_service {
> +     USIM_ACTV_SERVICE_FDN                           = 0,
> +     USIM_ACTV_SERVICE_BDN                           = 1,
> +     USIM_ACTV_SERVICE_ACL                           = 2
> +};
> +

Same thing here, SIM_EST_SERVICE would be better.

>  #define SIM_EFSPN_DC_HOME_PLMN_BIT 0x1
>  #define SIM_EFSPN_DC_ROAMING_SPN_BIT 0x2
>  

Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono

Reply via email to