I've been using the Intel compiler with warnings turned on. It
found a few problems that gcc did not point out. But our current
statecomp produces code that Intel will not compile. "gcc
-Wwrite-strings" fails with errors too.
The current statecomp produces, for example:
struct PINT_state_machine_s pvfs2_client_small_io_sm =
{
ST_setup_msgpairs,
"pvfs2_client_small_io_sm"
};
static union PINT_state_array_values ST_setup_msgpairs[] = {
(union PINT_state_array_values) "setup_msgpairs",
(union PINT_state_array_values) &pvfs2_client_small_io_sm,
(union PINT_state_array_values) 0,
(union PINT_state_array_values) small_io_setup_msgpairs,
(union PINT_state_array_values) 0,
(union PINT_state_array_values) ST_xfer_msgpairs,
(union PINT_state_array_values) -1,
(union PINT_state_array_values) 2
};
It is those union casts that cause problems.
I've switched things around to make it all void* instead. The types
don't actually matter since the state machine doesn't pay attention
to them, so this is an exercise more in fooling the compiler than
code correctness. With these changes, statecomp produces:
struct PINT_state_machine_s pvfs2_client_small_io_sm = {
(void *) ST_setup_msgpairs,
"pvfs2_client_small_io_sm"
};
static void *ST_setup_msgpairs[] = {
"setup_msgpairs",
&pvfs2_client_small_io_sm,
(void *) SM_NONE, small_io_setup_msgpairs,
/* return code */ (void *) 0,
/* => */ ST_xfer_msgpairs,
/* return code */ (void *) -1,
/* => */ (void *) SM_RETURN
};
Notice I've taken the opportunity to add some sugar in the form of
comments that may help people understand the statecomp output, and
have used the SM_ constants where possible. This part is orthogonal
to compilation, but since I had it torn apart...
Another possible way to fix this is by using named initializers,
like
static union PINT_state_array_values ST_setup_msgpairs[] = {
{ .state_name = "setup_msgpairs" },
{ .parent_machine = &pvfs2_client_small_io_sm },
{ .flag = SM_NONE },
...
which would be just dandy, but not all compilers have caught up with
this nice C99-ism. I didn't want to exclude old compilers, or
support two versions and add a configure check, so rejected this
idea.
The patch is attached. I'd like some feedback before applying it as
this has been a fragile area in the past that has seen a few
revisions. Comments?
-- Pete
Index: src/common/statecomp/codegen.c
===================================================================
RCS file: /projects/cvsroot/pvfs2/src/common/statecomp/codegen.c,v
retrieving revision 1.19
diff -u -p -r1.19 codegen.c
--- src/common/statecomp/codegen.c 14 Dec 2005 21:50:20 -0000 1.19
+++ src/common/statecomp/codegen.c 28 May 2006 16:52:54 -0000
@@ -35,24 +35,24 @@ void gen_init(void)
void gen_state_decl(char *state_name)
{
- fprintf(out_file,"static union PINT_state_array_values ST_%s[];\n",
state_name);
+ fprintf(out_file,"static void *ST_%s[];\n", state_name);
}
void gen_machine(char *machine_name,
char *first_state_name)
{
current_machine = machine_name;
- fprintf(out_file, "\nstruct PINT_state_machine_s %s =\n{\n\t",
machine_name);
- fprintf(out_file, "ST_%s,\n\t\"%s\"\n", first_state_name, machine_name);
- fprintf(out_file, "};\n");
+ fprintf(out_file, "\nstruct PINT_state_machine_s %s = {\n\t",
machine_name);
+ fprintf(out_file, "(void *) ST_%s,\n\t\"%s\"\n", first_state_name,
machine_name);
+ fprintf(out_file, "};\n\n");
}
void gen_state_start(char *state_name)
{
fprintf(out_file,
- "static union PINT_state_array_values ST_%s[] = {\n"
- "(union PINT_state_array_values) \"%s\",\n"
- "(union PINT_state_array_values) &%s,\n",
+ "static void *ST_%s[] = {\n"
+ "\t\"%s\",\n"
+ "\t&%s,\n",
state_name, state_name, current_machine);
}
@@ -65,20 +65,10 @@ void gen_state_action(char *run_func, in
{
switch (flag) {
case SM_NONE:
- fprintf(out_file,
- "(union PINT_state_array_values) %d",
- flag);
- fprintf(out_file,
- ",\n(union PINT_state_array_values) %s",
- run_func);
+ fprintf(out_file, "\t(void *) SM_NONE, %s", run_func);
break;
case SM_JUMP:
- fprintf(out_file,
- "(union PINT_state_array_values) %d",
- flag);
- fprintf(out_file,
- ",\n(union PINT_state_array_values) &%s",
- run_func);
+ fprintf(out_file, "\t(void *) SM_JUMP, &%s", run_func);
break;
default:
fprintf(stderr,
@@ -90,30 +80,22 @@ void gen_state_action(char *run_func, in
void gen_return_code(char *return_code)
{
- fprintf(out_file,
- ",\n(union PINT_state_array_values) %s",
- return_code);
+ fprintf(out_file, ",\n\t/* return code */ (void *) %s", return_code);
}
void gen_next_state(int flag, char *new_state)
{
switch (flag) {
case SM_NEXT:
- fprintf(out_file,
- ",\n(union PINT_state_array_values) ST_%s",
- new_state);
+ fprintf(out_file, ",\n\t/* => */ ST_%s", new_state);
break;
case SM_RETURN:
terminate_path_flag = 1;
- fprintf(out_file,
- ",\n(union PINT_state_array_values) %d",
- flag);
+ fprintf(out_file, ",\n\t/* => */ (void *) SM_RETURN");
break;
case SM_TERMINATE:
terminate_path_flag = 1;
- fprintf(out_file,
- ",\n(union PINT_state_array_values) %d",
- flag);
+ fprintf(out_file, ",\n\t/* => */ (void *) SM_TERMINATE");
break;
default:
fprintf(stderr,
Index: src/common/misc/state-machine-fns.h
===================================================================
RCS file: /projects/cvsroot/pvfs2/src/common/misc/state-machine-fns.h,v
retrieving revision 1.18
diff -u -p -r1.18 state-machine-fns.h
--- src/common/misc/state-machine-fns.h 14 Dec 2005 21:50:19 -0000 1.18
+++ src/common/misc/state-machine-fns.h 28 May 2006 16:52:54 -0000
@@ -78,7 +78,7 @@ static inline int PINT_state_machine_nex
int code_val = r->error_code; /* temp to hold the return code */
int retval; /* temp to hold return value of state action */
union PINT_state_array_values *loc; /* temp pointer into state memory */
- char * state_name;
+ const char * state_name;
char * machine_name;
do {
@@ -170,7 +170,7 @@ static inline int PINT_state_machine_inv
struct PINT_OP_STATE *s, job_status_s *r)
{
int retval;
- char * state_name;
+ const char * state_name;
char * machine_name;
state_name = PINT_state_machine_current_state_name(s);
Index: src/common/misc/state-machine.h
===================================================================
RCS file: /projects/cvsroot/pvfs2/src/common/misc/state-machine.h,v
retrieving revision 1.11
diff -u -p -r1.11 state-machine.h
--- src/common/misc/state-machine.h 14 Dec 2005 21:50:19 -0000 1.11
+++ src/common/misc/state-machine.h 28 May 2006 16:52:54 -0000
@@ -45,7 +45,7 @@
union PINT_state_array_values
{
- char *state_name;
+ const char *state_name;
struct PINT_state_machine_s *parent_machine;
int (*state_action)(struct PINT_OP_STATE *, job_status_s *);
int return_value;
_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
