Re: [PATCH 5/6] stkutil: Complete the TLV parsing/builder to support BIP commands

>> struct stk_network_access_name { >> - unsigned char name[127]; >> + unsigned char name[100]; >> unsigned char len; > > This part really makes no sense. Since you parse it into a string, > might as well just use that. All the above comments are just relevant. Thank you for having spent time to point all these remarks and especially all the coding style noncompliances. I will apply your comments in a new patch and separate cleaning/typo modifications in another dedicated patch. Here, the apn string is indeed extracted from the encoded buffer. As the encoded apn just can't exceeed 100 bytes as per the spec and as the extracted string is lesser than 100 bytes since we are removing the octet length field , I considered that the string buffer was better sized with 100 bytes than 127. But perhaps, I'm over thinking here...

> >> }; >> >> @@ -1250,6 +1298,21 @@ struct stk_command_launch_browser { >> char *text_passwd; >> }; >> >> +struct stk_command_open_channel { >> + char *alpha_id; >> + struct stk_icon_id icon_id; >> + struct stk_bearer_description bearer_desc; >> + unsigned short buf_size; >> + struct stk_network_access_name network_name; > > E.g. char *network_name; >

>> + /* The number of bytes remaining in the Rx buffer */ >> + unsigned int data_left_length; > > So maybe rx_remaining is a better name? > >> + /* >> + * The number of bytes of empty space available >> + * in the Tx buffer >> + */ >> + unsigned int empty_data_length; > > And tx_avail here? > > Then you don't need the comments ;) > > Also, why do you use unsigned int instead of unsigned char? > In practice, the size (rx_remaining or tx_avail) can reach 65535 as the buffer size it self given by the UICC is encoded by 2 bytes. I could consider indeed to use at least an unsigned short, but to be homogeneous with the type of the buffer size given by data.len, I preferred to go with an unsigned int.