hdlc: handle wrapped buffers
PATCH 1/1 hdlc: handle wrapped buffers
--- gatchat/gathdlc.c | 68 +++++++++++++++++++++++++++------------------------- 1 files changed, 35 insertions(+), 33 deletions(-)
gatchat/gathdlc.c +35 -33
@@ -53,11 +53,12 @@ static void new_bytes(GAtHDLC *hdlc) {unsigned int len =ring_buffer_len (hdlc->read_buffer);unsigned char *buf =ring_buffer_read_ptr (hdlc->read_buffer,0 ); +unsigned int wrap =ring_buffer_len_no_wrap (hdlc->read_buffer);unsigned char val;unsigned int pos =0 ;while (pos < len) { -if (buf[pos] ==0x7e ) { +if (*buf ==0x7e ) {if (hdlc->receive_func && hdlc->decode_offset >2 && hdlc->decode_fcs ==0xf0b8 ) { hdlc->receive_func(hdlc->decode_buffer,
@@ -68,22 +69,33 @@ static void new_bytes(GAtHDLC *hdlc) hdlc->decode_fcs =0xffff ; hdlc->decode_offset =0 ; pos++; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, + pos);continue ; } -if (buf[pos] ==0x7d ) { +if (*buf ==0x7d ) {if (pos +2 > len)break ; pos++; - val = buf[pos] ^0x20 ; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, + pos); + val = *buf ^0x20 ; }else - val = buf[pos]; + val = *buf; hdlc->decode_buffer[hdlc->decode_offset] = val; hdlc->decode_fcs =crc_ccitt_byte (hdlc->decode_fcs, val); hdlc->decode_offset++; pos++; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, pos); }ring_buffer_drain (hdlc->read_buffer, pos);
@@ -302,47 +314,37 @@ static void wakeup_write(GAtHDLC *hdlc) can_write_data, hdlc, write_watch_destroy); } -static inline void hdlc_put (GAtHDLC *hdlc,guint8 *buf,gsize *pos,guint8 c) +static inline void hdlc_buffer_put_char (struct ring_buffer *buffer, +const char val) { -gsize i = *pos; +ring_buffer_write (buffer, &val,1 ); +} +static inline void hdlc_put (GAtHDLC *hdlc,guint8 c) +{if (c ==0x7e || c ==0x7d ) { - buf[i++] =0x7d ; - buf[i++] = c ^0x20 ; +hdlc_buffer_put_char (hdlc->write_buffer,0x7d ); +hdlc_buffer_put_char (hdlc->write_buffer, c ^0x20 ); }else - buf[i++] = c; - - *pos = i; +hdlc_buffer_put_char (hdlc->write_buffer, c); }gboolean g_at_hdlc_send (GAtHDLC *hdlc,const unsigned char *data,gsize size) { -unsigned char *buf; -unsigned int space, i =0 ; +unsigned int i =0 ;guint16 fcs =0xffff ; -gsize pos; -do { - space =ring_buffer_avail_no_wrap (hdlc->write_buffer); -if (space ==0 ) -break ; +hdlc_buffer_put_char (hdlc->write_buffer,0x7e ); - buf =ring_buffer_write_ptr (hdlc->write_buffer); - pos =0 ; - -while (size--) { - fcs =crc_ccitt_byte (fcs, data[i]); -hdlc_put (hdlc, buf, &pos, data[i++]); - } - - fcs ^=0xffff ; -hdlc_put (hdlc, buf, &pos, fcs &0xff ); -hdlc_put (hdlc, buf, &pos, fcs >>8 ); - - buf[pos++] =0x7e ; +while (size--) { + fcs =crc_ccitt_byte (fcs, data[i]); +hdlc_put (hdlc, data[i++]); + } -ring_buffer_write_advance (hdlc->write_buffer, pos); - }while (0 ); + fcs ^=0xffff ; +hdlc_put (hdlc, fcs &0xff ); +hdlc_put (hdlc, fcs >>8 ); +hdlc_buffer_put_char (hdlc->write_buffer,0x7e );wakeup_write (hdlc);
Signature · 1 line
1.6.6.1
Hi Kristen,
gatchat/gathdlc.c static void new_bytes(GAtHDLC *hdlc) · line 56
Quoted 12 lines
gatchat/gathdlc.c | 68 +++++++++++++++++++++++++++------------------------- 1 files changed, 35 insertions(+), 33 deletions(-)gatchat/gathdlc.c +1
-0@@ -53,11 +53,12 @@ static void new_bytes(GAtHDLC *hdlc) {unsigned int len =ring_buffer_len (hdlc->read_buffer);unsigned char *buf =ring_buffer_read_ptr (hdlc->read_buffer,0 ); +unsigned int wrap =ring_buffer_len_no_wrap (hdlc->read_buffer);
to keep the design in sync with how gatchat.c does the variable order, please put wrap before *buf.
Quoted 36 lines
gatchat/gathdlc.c +12
-4unsigned char val;unsigned int pos =0 ;while (pos < len) { -if (buf[pos] ==0x7e ) { +if (*buf ==0x7e ) {if (hdlc->receive_func && hdlc->decode_offset >2 && hdlc->decode_fcs ==0xf0b8 ) { hdlc->receive_func(hdlc->decode_buffer,@@ -68,22 +69,33 @@ static void new_bytes(GAtHDLC *hdlc) hdlc->decode_fcs =0xffff ; hdlc->decode_offset =0 ; pos++; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, + pos);continue ; } -if (buf[pos] ==0x7d ) { +if (*buf ==0x7d ) {if (pos +2 > len)break ; pos++; - val = buf[pos] ^0x20 ; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, + pos); + val = *buf ^0x20 ; }else - val = buf[pos]; + val = *buf; hdlc->decode_buffer[hdlc->decode_offset] = val;
gatchat/gathdlc.c static void new_bytes(GAtHDLC *hdlc) · line 101
Quoted 10 lines
gatchat/gathdlc.c +3
-0hdlc->decode_fcs =crc_ccitt_byte (hdlc->decode_fcs, val); hdlc->decode_offset++; pos++; + buf++; +if (pos == wrap) + buf =ring_buffer_read_ptr (hdlc->read_buffer, pos); }ring_buffer_drain (hdlc->read_buffer, pos);
This part looks fine. However we repeat the following code here three times:
pos++; buf++; if (pos == wrap) buf = ring_buffer_read_ptr(hdlc->read_buffer, pos);
We might need to think about a bit of restructure or using a define.
gatchat/gathdlc.c static void wakeup_write(GAtHDLC *hdlc) · line 321
Quoted 11 lines
gatchat/gathdlc.c +4
-2@@ -302,47 +314,37 @@ static void wakeup_write(GAtHDLC *hdlc) can_write_data, hdlc, write_watch_destroy); } -static inline void hdlc_put (GAtHDLC *hdlc,guint8 *buf,gsize *pos,guint8 c) +static inline void hdlc_buffer_put_char (struct ring_buffer *buffer, +const char val) { -gsize i = *pos; +ring_buffer_write (buffer, &val,1 ); +}
Please don't do this. It has a massive performance impact. Please check the ring_buffer_write() function and see what it is doing. Just writing a char is bad. Really bad.
If you want to write a char then adding ring_buffer_write_char() function is the way to go here.
Quoted 44 lines
gatchat/gathdlc.c +10
-25+static inline void hdlc_put (GAtHDLC *hdlc,guint8 c) +{if (c ==0x7e || c ==0x7d ) { - buf[i++] =0x7d ; - buf[i++] = c ^0x20 ; +hdlc_buffer_put_char (hdlc->write_buffer,0x7d ); +hdlc_buffer_put_char (hdlc->write_buffer, c ^0x20 ); }else - buf[i++] = c; - - *pos = i; +hdlc_buffer_put_char (hdlc->write_buffer, c); }gboolean g_at_hdlc_send (GAtHDLC *hdlc,const unsigned char *data,gsize size) { -unsigned char *buf; -unsigned int space, i =0 ; +unsigned int i =0 ;guint16 fcs =0xffff ; -gsize pos; - do { - space = ring_buffer_avail_no_wrap(hdlc->write_buffer); - if (space == 0) - break; +hdlc_buffer_put_char (hdlc->write_buffer,0x7e ); - buf = ring_buffer_write_ptr(hdlc->write_buffer); - pos = 0; - - while (size--) { - fcs = crc_ccitt_byte(fcs, data[i]); - hdlc_put(hdlc, buf, &pos, data[i++]); - } - - fcs ^= 0xffff; - hdlc_put(hdlc, buf, &pos, fcs & 0xff); - hdlc_put(hdlc, buf, &pos, fcs >> 8); - - buf[pos++] = 0x7e; +while (size--) { + fcs =crc_ccitt_byte (fcs, data[i]); +hdlc_put (hdlc, data[i++]);
gatchat/gathdlc.c static void wakeup_write(GAtHDLC *hdlc) · line 349
Quoted 10 lines
gatchat/gathdlc.c +5
-2+ } -ring_buffer_write_advance (hdlc->write_buffer, pos); - }while (0 ); + fcs ^=0xffff ; +hdlc_put (hdlc, fcs &0xff ); +hdlc_put (hdlc, fcs >>8 ); +hdlc_buffer_put_char (hdlc->write_buffer,0x7e );wakeup_write (hdlc);
The only problem area with my original code is that when we are close before the wrapping and have to write an encoded character that has one byte before and one byte after the wrapping. Every other case should have just worked fine.
Regards
Marcel