c: Possible memory leak not detected

This issue was raised by byelipk in his exercise. I don’t think it applies in that case, but ran again using malloc and no error, I’m not used to Unity yet, but I think there should be one for a malloc without free. Creating the issue until I can dig deeper.

Code I ran to test:

#include "raindrops.h"
#include "stdio.h"
#include "string.h"
#include <stdlib.h>

#define PLING_FACTOR 3
#define PLANG_FACTOR 5
#define PLONG_FACTOR 7

char *convert(char *buffer, int buffer_length, int drops)
{
   memset(buffer, '\0', sizeof(char) * buffer_length);
   if ((drops % PLING_FACTOR != 0) &&
       (drops % PLANG_FACTOR != 0) && (drops % PLONG_FACTOR != 0)) {
      snprintf(buffer, buffer_length, "%d", drops);
   } else {
      snprintf
          (buffer,
           buffer_length,
           "%s%s%s",
           drops % PLING_FACTOR == 0 ? "Pling" : "",
           drops % PLANG_FACTOR == 0 ? "Plang" : "",
           drops % PLONG_FACTOR == 0 ? "Plong" : "");
   }
    char* newbuffer = malloc(sizeof(char) * buffer_length);
    memcpy(newbuffer,buffer,buffer_length);
   return newbuffer;
}

Completely unrelated to the title, but raindrops_test.c needs to be updated to check the value in the buffer, not just the return value. They should match. I can address that at the same time or with a different patch. However you want.

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 27 (27 by maintainers)

Most upvoted comments

I see no reason to not to allow the target for Mac,They should be either able to modify their default suppression file or alias valgrind to vallgrind --supress…

On Fri, Nov 10, 2017 at 3:45 PM, toby notifications@github.com wrote:

Both. Can’t immediately recall why brew didn’t work out but it seems that the gcc that was on the machine wasn’t suitable for building valgrind according to what configure.sh reported. I did try to figure out a way around it but A) my time on the machine was limited (wasn’t my rig) and I was wary of going down the rabbit hole of installing a never ending list of things just to fix the last thing 😅 , B) I’m so unused to macs (I’m a life-long windows user and very occasional Linux dabbler) that every new problem makes things exponentially harder.

I guess if we want to fix the Mac support that could be a future issue/PR? Having Just Windows and Linux support for now is better than the current no-support 😄 .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/exercism/c/issues/169#issuecomment-343581283, or mute the thread https://github.com/notifications/unsubscribe-auth/AFkhSiYjReuO39qZUmLzOzsv1TYRt_OZks5s1LXngaJpZM4PZfCq .

Gamecock on Nov 10, 2017

Yes, I have 1 uninitialized bytes and 1 possible leak with the darwin16.supp installed. Either the darwin.supp needs to be updated, or I have something else corrupting my machine.

If you install valgrind with brew the darwin16.supp is installed as part of your default.

Gamecock on Nov 10, 2017

I’m adding some tags here and describing what this is looking for:

Basically, this thread is saying it would be good to add a new make target to run the tests under Valgrind. With extra credit if you can have it pretty print whether or not there were errors, and re-use the target in bin/run-tests

A minimal case:

memcheck: tests.out
    @echo Running memory test
    valgrind --tool=memcheck ./tests.out

arcuru on Oct 7, 2017

Right, Unity doesn’t support this. CppUTest does support it, but breaks frequently enough with updated toolchains that I would rather not go this route.

@patricksjackson what do you think about possibly making valgrind an optional install that our makefiles can use if it is available on the user’s system?

ryanplusplus on Sep 15, 2017