Re: thread safe usage of multiple phones

Pawel Kot <[email protected]> Sun, 14 May 2017 20:53:09 +0200
Newsgroups gmane.linux.drivers.gnokii
Message-ID <CAHytCw7PHKCkcZwXt+JhFYb3Rj=YXUZnsM6BKk8t0GK5_bnb6g@mail.gmail.com> 
Well, it looks like the solution I have implemented was quite dumb.
This static variable was really not necessary (however it made
solution simpler). Can you please try the attached patch and say
whether it works for you? It is against git version.

take care,
Pawel

On Sun, May 14, 2017 at 1:29 PM, Pawel Kot <[email protected]> wrote:
> Hi Peter,
>
> On Sun, May 14, 2017 at 9:31 AM, Peter Koch <[email protected]> wrote:
>> Have a look at routine verify_max_message_len() in
>> common/links/fbus-phonet.c, starting at line 64
>
> Yeah, that makes sense. I have reviewed just statemachine and sms
> code. And they seem thread safe. I have forgotten the link code. It
> indeed assumes communication with one device.
>
>> Of course this only happens when the phonet-driver is used, so my
>> serial nokia 6310 works fine.
>
> I will have a closer look.
>
>> static int verify_max_message_len(int len, char **message_buffer)
>> {
>>         static int max_message_len = 0;
>>
>>         if(len<PHONET_FRAME_MAX_LENGTH) len=PHONET_FRAME_MAX_LENGTH;
>>         if(len>PHONET_FRAME_MAX_LENGTH || !*message_buffer){
>>                 dprintf("reallocating message_buffer to %d bytes\n", len+1);
>>                 *message_buffer = realloc(*message_buffer, len+1);
>>         }
>>         return *message_buffer ? len+1 : 0;
>
> Quite frankly at the moment I'm not sure why would it work.
>
>>         if (len > max_message_len) {
>>                 dprintf("overrun: %d %d\n", len, max_message_len);
>>                 *message_buffer = realloc(*message_buffer, len + 1);
>>                 max_message_len = len + 1;
>>         }
>>         if (*message_buffer)
>>                 return max_message_len;
>>         else
>>                 return 0;
>> }
>
> Take care,
> Pawel



-- 
Pawel Kot

_______________________________________________
gnokii-users mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
fbusphonet.patch (application/octet-stream, 2.3 KB) 
diff --git a/common/links/fbus-phonet.c b/common/links/fbus-phonet.c
index 3300cea..d50df0b 100644
--- a/common/links/fbus-phonet.c
+++ b/common/links/fbus-phonet.c
@@ -47,10 +47,8 @@ static gn_error phonet_send_message(unsigned int messagesize, unsigned char mess
 
 /*--------------------------------------------*/
 
-static int verify_max_message_len(int len, char **message_buffer)
+static int verify_max_message_len(int max_message_len, int len, char **message_buffer)
 {
-	static int max_message_len = 0;
-
 	if (len > max_message_len || !*message_buffer) {
 		dprintf("overrun, reallocating: %d %d\n", len, max_message_len);
 		*message_buffer = realloc(*message_buffer, len + 1);
@@ -171,7 +169,7 @@ static void phonet_rx_statemachine(unsigned char rx_byte, struct gn_statemachine
 		i->message_length = i->message_length + rx_byte;
 		i->state = FBUS_RX_GetMessage;
 		i->buffer_count = 0;
-		if (!verify_max_message_len(i->message_length, &(i->message_buffer))) {
+		if ((i->malloced = verify_max_message_len(i->malloced, i->message_length, &(i->message_buffer))) == 0) {
 			dprintf("PHONET: Failed to allocate memory for larger buffer\n");
 			i->message_corrupted = 1;
 		}
@@ -369,6 +367,7 @@ static void phonet_cleanup(struct gn_statemachine *state)
 {
 	free(FBUSINST(state)->message_buffer);
 	FBUSINST(state)->message_buffer = NULL;
+	FBUSINST(state)->malloced = 0;
 }
 
 /* Initialise variables and start the link */
@@ -388,7 +387,7 @@ gn_error phonet_initialise(struct gn_statemachine *state)
 	if ((FBUSINST(state) = calloc(1, sizeof(phonet_incoming_message))) == NULL)
 		return GN_ERR_MEMORYFULL;
 
-	if (!verify_max_message_len(PHONET_FRAME_MAX_LENGTH, &(FBUSINST(state)->message_buffer))) {
+	if ((FBUSINST(state)->malloced = verify_max_message_len(0, PHONET_FRAME_MAX_LENGTH, &(FBUSINST(state)->message_buffer))) == 0) {
 		dprintf("PHONET: Failed to initalize initial incoming buffer for %d bytes\n", PHONET_FRAME_MAX_LENGTH);
 		return GN_ERR_MEMORYFULL;
 	}
diff --git a/include/links/fbus-phonet.h b/include/links/fbus-phonet.h
index 89eaf0b..36c55c0 100644
--- a/include/links/fbus-phonet.h
+++ b/include/links/fbus-phonet.h
@@ -49,6 +49,7 @@ typedef struct {
 	int message_length;
 	char *message_buffer;
 	int message_corrupted;
+	int malloced;
 } phonet_incoming_message;
 
 #endif   /* #ifndef _gnokii_links_fbus_phonet_h */