On Tue, Dec 30, 2008 at 04:26:46PM +0530, Sudhir Kumar wrote:
> 
> Earlier we were creating the messgae array dynamicaly which is not required
> as the messages do not change dynamicaly. Let us have a static 2D array of
> messages. This patch does that.
> (This addresses the comment on the list ;))

OK, this looks good :). Juat a minor nit.

> 
> Signed-off-by: Sudhir Kumar <[email protected]>
> 
> Index: trunk/tests/libcgrouptest.h
> ===================================================================
> --- trunk.orig/tests/libcgrouptest.h
> +++ trunk/tests/libcgrouptest.h
> @@ -78,8 +78,8 @@ enum info_message_t {
>       NOMESSAGE,
>  };
> 
> -/* Create a matrix of possible info messages */
> -char **info;
> +/* The set of verbose messages useful to the user */
> +extern char info[NUM_MSGS][SIZE];
> 
>  int64_t val_int64;
>  u_int64_t val_uint64;
> @@ -135,11 +135,6 @@ static inline void message(int num, int 
>  static inline void build_path(char *target, char *mountpoint,
>                                const char *group, const char *file);
> 
> -/* Allocate memory and populate info messages */
> -void set_info_msgs();
> -/* Free the allocated memory for info messages */
> -void free_info_msgs();
> -
>  static inline pid_t cgrouptest_gettid()
>  {
>       return syscall(__NR_gettid);
> Index: trunk/tests/libcgrouptest01.c
> ===================================================================
> --- trunk.orig/tests/libcgrouptest01.c
> +++ trunk/tests/libcgrouptest01.c
> @@ -16,6 +16,30 @@
> 
>  #include "libcgrouptest.h"
> 
> +/* The messages that may be useful to the user */
> +char info[NUM_MSGS][SIZE] = {
> +     /* NULLGRP */ " Parameter nullcgroup\n", /* 0 */
> +     /* COMMONGRP */ " Parameter commoncgroup\n",
> +     /* NOTCRTDGRP */ " Parameter not created group\n",
> +     /* SAMEGRP */ " Parameter same cgroup\n",
> +     /* TASKINGRP */ " Task found in group/s\n",
> +     /* TASKNOTINGRP */ " Task not found in group/s\n",
> +     /* TASKNOTINANYGRP */ " Task not found in all groups\n",
> +     /* GRPINFS */ " group found in filesystem\n",
> +     /* GRPNOTINFS */ " group not found in filesystem\n",
> +     /* GRPINBOTHCTLS */ " group found under both controllers\n",
> +     /* GRPNOTIN2NDCTL */  " group not found under second controller\n",
> +     /* GRPNOTIN1STCTL */ " group not found under first controller\n",
> +     /* GRPMODINBOTHCTLS */ " group modified under both controllers\n",
> +     /* GRPNOTMODIN2NDCTL */ " group not modified under second controller\n",
> +     /* GRPNOTMODINANYCTL */ " group not modified under any controller\n",
> +     /* GRPDELETEDINFS */ " Group deleted from filesystem\n",
> +     /* GRPNOTDELETEDINFS */ " Group not deleted from filesystem\n",
> +     /* GRPNOTDELETEDGLOBALY */ " Group not deleted globally\n",
> +     /* In case there is no extra info messages to be printed */
> +     /* NOMESSAGE */ "\n",   /* 18 */
> +     };
> +

Can you do the messages on the left, and have a uniform indent for the
comment? This is hard to read. (I guess for these comments we should be
able to ignore the coding style simply because it becomes more readable
then. Balbir, any comments on that?)

Thanks,
-- 
regards,
Dhaval

------------------------------------------------------------------------------
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to