On 09/27/2016 05:17 PM, Steven Toth wrote:
On Fri, Sep 23, 2016 at 12:19 PM, Brian Paul <bri...@vmware.com> wrote:
Hi Steven,
I did a more thorough review per your request...
Thank you Brian.
All of your suggestions have been implemented, and new patches pushed to the ML.
Were you planning on squashing the two patches? I think you should.
BTW, I see that some of the new changes could use const qualifiers. For
example:
@@ -97,10 +97,10 @@ find_dsi_by_name(char *n, int mode)
}
static int
-get_file_values(struct diskstat_info *dsi, struct stat_s *s)
+get_file_values(char *fn, struct stat_s *s)
That could be 'const char *fn'. Same thing in other places.
Sorry to be nit picky about const qualifiers, but they're pretty helpful
when reading code to help understand which arguments may or may not be
modified by a function.
...with the exception of one, primarily because I wanted to comment.
+#if HAVE_LIBSENSORS
+ else if (sscanf(name, "sensors_temp_cu-%s", arg_name) == 1) {
+ hud_sensors_temp_graph_install(pane, &name[16],
What's the significance of name[16]? Should that be a #define ?
Everything after the hyphen is essentially its unique sensor name,
prior to the hyphen is a routing string that tells mesa HUD us to use
the lmsensor HUD module, rather than say... the disk stats module.
So 16, is the length of "sensors_temp_cu-" and we pass the remainder
into the sensor specific initializer func - which is all it cares
about.
I'm happy to implement whatever the project recommends, so are you
suggesting instead:
#define SOMEPREFIX "sensors_temp_cu-"
then
hud_sensors_temp_graph_install(pane, &name[sizeof(SOMEPREFIX - 1)]
Or, have I misunderstood your comment?
Thanks for explaining. But why not do this?
else if (sscanf(name, "sensors_temp_cu-%s", arg_name) == 1) {
hud_sensors_temp_graph_install(pane, arg_name, ...
as you did in other places?
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
