0

context:https://stackoverflow.com/a/50655730/15603477

#include <stdio.h>
#include <stdlib.h>
#include<string.h>
#include<ctype.h>
#include<limits.h>
#include<uchar.h>
#include<assert.h>
#define MAXW    128
#define MAXC    112
int main(int argc,char **argv)
{
    int readdef = 0;            /* flag for reading definition */
    size_t offset = 0            /* offset for each part of definition */
        ,len = 0;             /* length of each line */
    char buf[MAXC] = "";      /* read (line) buffer */
    char word[MAXW] = "";     /* buffer storing word */
    char defn[MAXC] = "";     /* buffer storing definition */

    /* open filename given as 1st argument, (or read stdin by default) */
    FILE *fp = argc > 1 ? fopen(argv[1],"r") : stdin;

    if(!fp){     /* validate file open for reading */
        fprintf(stderr,"error: file '%s' open failed\n",argv[1]);
        exit(EXIT_FAILURE);
    }

    while(fgets(buf,MAXC,fp))
    {
        char *p = buf;   /* pointer to parse word & 1st part of defn */

        if(*buf == '\n') {
            defn[offset-1] = 0;
            printf("defn:%s\n\n",defn);
            readdef = 0;
            offset= 0;
        }
        else if(readdef == 0)
        {
            while(*p && *p != '.') p++;
            if(p-buf+1 > MAXW){
                fprintf(stderr,"error: word exceeds %d chars.\n",MAXW-1);
                exit(EXIT_FAILURE);
            }
            snprintf(word,p-buf+1,"%s",buf);     /* copy to word */
            printf("word=%s|\n",word);
            
            while(ispunct(*p) || isspace(*p))
                p++;
            
            len = strlen(p);

            if(len && p[len-1] == '\n')
                p[len-1] = ' ';

            strcpy(defn,p);
            offset +=len;
            readdef = 1;
        }
        else{
            len = strlen(buf);              /*get the line lenfth */
            if(len && buf[len-1] == '\n')   /* chk \n, overwite w/' ' */
                buf[len-1] = ' ';
            
            if(offset + len + 1 > MAXC){
                fprintf(stderr,"error: definition exceed %d chars\n",MAXC-1);
                // free(buf);
                exit(EXIT_FAILURE);
            }

            snprintf(defn+offset,len+1,"%s",buf);   /* append to defn */
            offset += len;                          /*update offset*/
        }
    }

    if(fp != stdin) fclose(fp);

    defn[offset - 1] = 0;
    printf("defn: %s\n\n",defn);

    exit(EXIT_SUCCESS);
}

valgrind info.

error: definition exceed 111 chars
==28017==
==28017== HEAP SUMMARY:
==28017==     in use at exit: 472 bytes in 1 blocks
==28017==   total heap usage: 3 allocs, 2 frees, 5,592 bytes allocated
==28017==
==28017== 472 bytes in 1 blocks are still reachable in loss record 1 of 1
==28017==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==28017==    by 0x48ED6CD: __fopen_internal (iofopen.c:65)
==28017==    by 0x48ED6CD: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==28017==    by 0x109285: main (in /home/jian/helloc/a.out)
==28017==
==28017== LEAK SUMMARY:
==28017==    definitely lost: 0 bytes in 0 blocks
==28017==    indirectly lost: 0 bytes in 0 blocks
==28017==      possibly lost: 0 bytes in 0 blocks
==28017==    still reachable: 472 bytes in 1 blocks
==28017==         suppressed: 0 bytes in 0 blocks
==28017==
==28017== For lists of detected and suppressed errors, rerun with: -s
==28017== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

if MAXC is larger enough, then no LEAK SUMMARY. however if it's smaller, then there is memory leak error.
How can I fix the memory leak error when MAXC is not large enough to hold the string.

I also wonder even if dat/definitions.txt first line is empty new line, then defn[offset-1] = 0; would be defn[-1] = 0; but gcc still no error. should i expect error or warning like above array bounds of array char[112]?

jian
  • 4,119
  • 1
  • 17
  • 32
  • 2
    On an unrelated issue, why do size-calculation in `snprintf(word,p-buf+1,"%s",buf)`, when the size should really be `sizeof word`? – Some programmer dude Oct 28 '22 at 11:45
  • 2
    I see a `// free(buf);`. `free(buf);` is totally wrong, you only can call `free` on a pointer that has been returned by `malloc` and relatives. – Jabberwocky Oct 28 '22 at 11:45
  • 3
    Also, what if the very first line is empty, or the file itself is empty? Then `defn[offset-1]` will use `-1` as index. – Some programmer dude Oct 28 '22 at 11:46
  • I'd start putting some asserts to check if nones of your array indexes goes out of bounds – Jabberwocky Oct 28 '22 at 11:47
  • 1
    As for your "leak", it's a false positive because of how `fopen` creates its structures and buffers for the file. The memory will be released by the operating system as the program terminates. – Some programmer dude Oct 28 '22 at 11:48
  • 1
    As for the rest of the code, it seems overly complex for just reading some text from a file. What is the contents of the file you're supposed to read? What kind of processing are you supposed to do with that content? – Some programmer dude Oct 28 '22 at 11:52
  • @Someprogrammerdude I guess the questions would be why there is an difference when MAXC is not larger enough? and is defn[-1] is legal? – jian Oct 28 '22 at 12:21
  • 1
    @jian Valid indices for an array access are `[0, LENGTH - 1]`. Anything is an out-of-bounds memory access that will invoke [Undefined Behaviour](https://en.cppreference.com/w/c/language/behavior). – Oka Oct 28 '22 at 12:33

2 Answers2

1

The Valgrind report shows that the memory that remains allocated at program exit (but is still reachable, so is not leaked per se), was allocated by fopen(). Presumably, this will be released if you fclose() the stream that was opened, which is pointed to by fp.

Alternatively, just don't worry about it. The file will be closed and the memory released at program termination. Nothing has actually been leaked, as all the allocated memory remains accessible to the program until its termination.

Note: you may fclose(fp) even if fp is a copy of stdin, as long as the program does not afterward attempt to read anything from its standard input.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
0

buf does not need to be freed here.

if(offset + len + 1 > MAXC){
            fprintf(stderr,"error: definition exceed %d chars\n",MAXC-1);
            // free(buf);//If you are calling this...
            exit(EXIT_FAILURE);
        }

...it would be a mistake as buf is shown to be created on the stack as an array: char buf[MAXC] = "";, not as a pointer to memory on the heap. (eg: char *buf = malloc(MAXC);)

Calling fclose(fp) when you are finished with the file pointer will free the memory cited in valgrind.

Also consider the case where the first line of your file may contain just a newline. This code section

if(*buf == '\n') {
        defn[offset-1] = 0;

results in negative array index.

Finally, a simple way to handle a newline after calling fgets() is to just eliminate it:

while(fgets(buf,MAXC,fp))
{
    buf[strcspn(buf, "\n")] = 0;//deletes newline, preserves NULL termination
    char *p = buf;   /* pointer to parse word & 1st part of defn */

    //if(*buf == '\n') {//no longer needed
ryyker
  • 22,849
  • 3
  • 43
  • 87