Properly handle RichString_extendLen when an external buffer already exists

[BenBE]

The leak was found using GCC 14 and -fanalyzer -flto:

RichString.c: In function ‘RichString_extendLen’:
RichString.c:25:22: warning: leak of ‘this_25(D)->chptr’ [CWE-401] [-Wanalyzer-malloc-leak]
   25 |          this->chptr = xMalloc(charBytes(len + 1));
      |                      ^
  ‘ZramMeter_display’: events 1-2
    |
    |linux/ZramMeter.c:51:13:
    |   51 | static void ZramMeter_display(const Object* cast, RichString* out) {
    |      |             ^
    |      |             |
    |      |             (1) entry to ‘ZramMeter_display’
    |......
    |   61 |    RichString_appendAscii(out, CRT_colors[METER_TEXT], " used:");
    |      |    ~         
    |      |    |
    |      |    (2) calling ‘RichString_appendAscii’ from ‘ZramMeter_display’
    |
    +--> ‘RichString_appendAscii’: events 3-4
           |
           |RichString.c:246:5:
           |  246 | int RichString_appendAscii(RichString* this, int attrs, const char* data) {
           |      |     ^
           |      |     |
           |      |     (3) entry to ‘RichString_appendAscii’
           |  247 |    return RichString_writeFromAscii(this, attrs, data, this->chlen, strlen(data));
           |      |           ~
           |      |           |
           |      |           (4) calling ‘RichString_writeFromAscii’ from ‘RichString_appendAscii’
           |
           +--> ‘RichString_writeFromAscii’: events 5-6
                  |
                  |  135 | static inline int RichString_writeFromAscii(RichString* this, int attrs, const char* data, int from, int len) {
                  |      |                   ^
                  |      |                   |
                  |      |                   (5) entry to ‘RichString_writeFromAscii’
                  |  136 |    int newLen = from + len;
                  |  137 |    RichString_setLen(this, newLen);
                  |      |    ~               
                  |      |    |
                  |      |    (6) calling ‘RichString_setLen’ from ‘RichString_writeFromAscii’
                  |
                  +--> ‘RichString_setLen’: events 7-8
                         |
                         |   42 | static void RichString_setLen(RichString* this, int len) {
                         |      |             ^
                         |      |             |
                         |      |             (7) entry to ‘RichString_setLen’
                         |......
                         |   47 |       RichString_extendLen(this, len);
                         |      |       ~      
                         |      |       |
                         |      |       (8) calling ‘RichString_extendLen’ from ‘RichString_setLen’
                         |
                         +--> ‘RichString_extendLen’: events 9-10
                                |
                                |   22 | static void RichString_extendLen(RichString* this, int len) {
                                |      |             ^
                                |      |             |
                                |      |             (9) entry to ‘RichString_extendLen’
                                |......
                                |   34 |          this->chptr = xRealloc(this->chptr, charBytes(len + 1));
                                |      |                        ~
                                |      |                        |
                                |      |                        (10) calling ‘xRealloc’ from ‘RichString_extendLen’
                                |
                                +--> ‘xRealloc’: events 11-13
                                       |
                                       |XUtils.c:64:7:
                                       |   64 | void* xRealloc(void* ptr, size_t size) {
                                       |      |       ^
                                       |      |       |
                                       |      |       (11) entry to ‘xRealloc’
                                       |   65 |    assert(size > 0);
                                       |   66 |    void* data = size ? realloc(ptr, size) : NULL; // deepcode ignore MemoryLeakOnRealloc: this goes to fail()
                                       |      |                        ~
                                       |      |                        |
                                       |      |                        (12) allocated here
                                       |      |                        (13) when ‘realloc’ succeeds, moving buffer
                                       |
                                <------+
                                |
                              ‘RichString_extendLen’: event 14
                                |
                                |RichString.c:34:24:
                                |   34 |          this->chptr = xRealloc(this->chptr, charBytes(len + 1));
                                |      |                        ^
                                |      |                        |
                                |      |                        (14) returning to ‘RichString_extendLen’ from ‘xRealloc’
                                |
                         <------+
                         |
                       ‘RichString_setLen’: event 15
                         |
                         |   47 |       RichString_extendLen(this, len);
                         |      |       ^
                         |      |       |
                         |      |       (15) returning to ‘RichString_setLen’ from ‘RichString_extendLen’
                         |
                  <------+
                  |
                ‘RichString_writeFromAscii’: event 16
                  |
                  |  137 |    RichString_setLen(this, newLen);
                  |      |    ^
                  |      |    |
                  |      |    (16) returning to ‘RichString_writeFromAscii’ from ‘RichString_setLen’
                  |
           <------+
           |
         ‘RichString_appendAscii’: event 17
           |
           |  247 |    return RichString_writeFromAscii(this, attrs, data, this->chlen, strlen(data));
           |      |           ^
           |      |           |
           |      |           (17) returning to ‘RichString_appendAscii’ from ‘RichString_writeFromAscii’
           |
    <------+
    |
  ‘ZramMeter_display’: events 18-19
    |
    |linux/ZramMeter.c:61:4:
    |   61 |    RichString_appendAscii(out, CRT_colors[METER_TEXT], " used:");
    |      |    ^
    |      |    |
    |      |    (18) returning to ‘ZramMeter_display’ from ‘RichString_appendAscii’
    |   62 |    RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer);
    |      |    ~
    |      |    |
    |      |    (19) calling ‘RichString_appendAscii’ from ‘ZramMeter_display’
    |
    +--> ‘RichString_appendAscii’: events 20-21
           |
           |RichString.c:246:5:
           |  246 | int RichString_appendAscii(RichString* this, int attrs, const char* data) {
           |      |     ^
           |      |     |
           |      |     (20) entry to ‘RichString_appendAscii’
           |  247 |    return RichString_writeFromAscii(this, attrs, data, this->chlen, strlen(data));
           |      |           ~
           |      |           |
           |      |           (21) calling ‘RichString_writeFromAscii’ from ‘RichString_appendAscii’
           |
           +--> ‘RichString_writeFromAscii’: events 22-23
                  |
                  |  135 | static inline int RichString_writeFromAscii(RichString* this, int attrs, const char* data, int from, int len) {
                  |      |                   ^
                  |      |                   |
                  |      |                   (22) entry to ‘RichString_writeFromAscii’
                  |  136 |    int newLen = from + len;
                  |  137 |    RichString_setLen(this, newLen);
                  |      |    ~               
                  |      |    |
                  |      |    (23) calling ‘RichString_setLen’ from ‘RichString_writeFromAscii’
                  |
                  +--> ‘RichString_setLen’: events 24-25
                         |
                         |   42 | static void RichString_setLen(RichString* this, int len) {
                         |      |             ^
                         |      |             |
                         |      |             (24) entry to ‘RichString_setLen’
                         |......
                         |   47 |       RichString_extendLen(this, len);
                         |      |       ~      
                         |      |       |
                         |      |       (25) calling ‘RichString_extendLen’ from ‘RichString_setLen’
                         |
                         +--> ‘RichString_extendLen’: events 26-27
                                |
                                |   22 | static void RichString_extendLen(RichString* this, int len) {
                                |      |             ^
                                |      |             |
                                |      |             (26) entry to ‘RichString_extendLen’
                                |......
                                |   25 |          this->chptr = xMalloc(charBytes(len + 1));
                                |      |                      ~
                                |      |                      |
                                |      |                      (27) ‘this_25(D)->chptr’ leaks here; was allocated at (12)
                                |

[fasterit]

Looks good otherwise

[fasterit]

Does not ensure trailing \NUL does it?

[cgzones]

RichString_setChar(this, len, 0); on line 38 will do this

[BenBE]

Exactly. NUL termination is handled below the conditional re-allocations in a central place.

[fasterit]

Does not ensure trailing \NUL does it?

[BenBE]

Handled by:

   RichString_setChar(this, len, 0);

below the large if statement.

[cgzones]

RichString_setChar(this, len, 0); on line 38 will do this