[Pidgin] #7282: some unnecessary checking in circbuffer implementation

[Pidgin]

#7282: some unnecessary checking in circbuffer implementation
------------------------+---------------------------------------------------
 Reporter:  rfan.cn     |        Owner:  datallah 
     Type:  defect      |       Status:  new      
Milestone:              |    Component:  libpurple
  Version:  2.5.1       |   Resolution:           
 Keywords:  circbuffer  |  
------------------------+---------------------------------------------------

Old description:

> There is a unnecessary checking in circbuffer.c. Look at this section of
> codes:
>

> {{{
>
> 0069 void oul_circ_buffer_append(OulCircBuffer *buf, gconstpointer src,
> gsize len) {
> ...
> 0083    if (buf->inptr >= buf->outptr)
> 0084            len_stored = MIN(len, buf->buflen
> 0085                    - (buf->inptr - buf->buffer));
> 0086    else
> 0087            len_stored = len;
> 0088
> 0089    memcpy(buf->inptr, src, len_stored);
> 0090
> 0091    if (len_stored < len) {
> 0092            memcpy(buf->buffer, (char*)src + len_stored, len -
> len_stored);
> 0093            buf->inptr = buf->buffer + (len - len_stored);
> 0094    } else if ((buf->buffer - buf->inptr) == len_stored) {
> 0095            buf->inptr = buf->buffer;
> 0096    } else {
> 0097            buf->inptr += len_stored;
> 0098    }
> ...
> }
>
> }}}
>
> In line 0094, the if logic will be always false as buf->intr will be
> greater than buf->buffer all the time. In other words it means the if
> else always false, and in fact as from line 0083 to 0087, it make sure
> the len_stored <= len, no need to add that if else sentence. It should be
> cleaned as following:
>
> {{{
>
> 0069 void oul_circ_buffer_append(OulCircBuffer *buf, gconstpointer src,
> gsize len) {
> ...
> 0083    if (buf->inptr >= buf->outptr)
> 0084            len_stored = MIN(len, buf->buflen
> 0085                    - (buf->inptr - buf->buffer));
> 0086    else
> 0087            len_stored = len;
> 0088
> 0089    memcpy(buf->inptr, src, len_stored);
> 0090
> 0091    if (len_stored < len) {
> 0092            memcpy(buf->buffer, (char*)src + len_stored, len -
> len_stored);
> 0093            buf->inptr = buf->buffer + (len - len_stored);
> 0094    } else {
> 0095            buf->inptr += len_stored;
> 0096    }
> ...
> }
>
> }}}

New description:

 There is a unnecessary checking in circbuffer.c. Look at this section of
 codes:


 {{{

 0069 void purple_circ_buffer_append(PurpleCircBuffer *buf, gconstpointer
 src, gsize len) {
 ...
 0083    if (buf->inptr >= buf->outptr)
 0084            len_stored = MIN(len, buf->buflen
 0085                    - (buf->inptr - buf->buffer));
 0086    else
 0087            len_stored = len;
 0088
 0089    memcpy(buf->inptr, src, len_stored);
 0090
 0091    if (len_stored < len) {
 0092            memcpy(buf->buffer, (char*)src + len_stored, len -
 len_stored);
 0093            buf->inptr = buf->buffer + (len - len_stored);
 0094    } else if ((buf->buffer - buf->inptr) == len_stored) {
 0095            buf->inptr = buf->buffer;
 0096    } else {
 0097            buf->inptr += len_stored;
 0098    }
 ...
 }

 }}}

 In line 0094, the if logic will be always false as buf->intr will be
 greater than buf->buffer all the time. In other words it means the if else
 always false, and in fact as from line 0083 to 0087, it make sure the
 len_stored <= len, no need to add that if else sentence. It should be
 cleaned as following:

 {{{

 0069 void purple_circ_buffer_append(PurpleCircBuffer *buf, gconstpointer
 src, gsize len) {
 ...
 0083    if (buf->inptr >= buf->outptr)
 0084            len_stored = MIN(len, buf->buflen
 0085                    - (buf->inptr - buf->buffer));
 0086    else
 0087            len_stored = len;
 0088
 0089    memcpy(buf->inptr, src, len_stored);
 0090
 0091    if (len_stored < len) {
 0092            memcpy(buf->buffer, (char*)src + len_stored, len -
 len_stored);
 0093            buf->inptr = buf->buffer + (len - len_stored);
 0094    } else {
 0095            buf->inptr += len_stored;
 0096    }
 ...
 }

 }}}

--

Comment(by rfan.cn):

 Sorry, I changed function name by mistake, fix it now ;)

-- 
Ticket URL: <http://developer.pidgin.im/ticket/7282#comment:3>
Pidgin <http://pidgin.im>
Pidgin