D5381: Add brightness control using ddcutil lib

[David Edmundson]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170526/6786f5df/attachment-0001.html>