davidedmundson added a comment.

  Looks good, I think I just have one comment about a leak, but the docs are 
confusing, so it's possible I'm wrong.

INLINE COMMENTS

> ddcutilbrightness.cpp:54
> +
> +        rc = ddca_create_dispno_display_identifier(iDisp+1, &did); // 
> ddcutil uses 1 paded indexing for displays
> +

this needs a ddca_free_display_identifier I think?

> ddcutilbrightness.cpp:134
> +        }
> +        //is this needed ? are variables created in the method automatically 
> free'd ?
> +        ddca_free_parsed_capabilities(parsedCapabilities);

yes it is needed.

>From the docs:

- It is the responsibility of the caller to free the returned struct
- using ddca_free_parsed_capabilities().

but generally speaking with C APIs if you get something that takes a pointer to 
a pointer, it's creating a new object.

REVISION DETAIL
  https://phabricator.kde.org/D5381

To: dvogel, broulik
Cc: strobach, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

Reply via email to