On Jul 16, 2007  11:43 -0400, Weikuan Yu wrote:
> --- romio-orig/adio/ad_lustre/ad_lustre_rwcontig.c    1969-12-31 
> 19:00:00.000000000 -0500
> +++ romio-ornl/adio/ad_lustre/ad_lustre_rwcontig.c    2007-07-14 
> 08:33:18.000000000 -0400
> @@ -0,0 +1,85 @@
> +static void ADIOI_LUSTRE_IOContig(ADIO_File fd, void *buf, int count, 
> +                   MPI_Datatype datatype, int file_ptr_type,
> +                ADIO_Offset offset, ADIO_Status *status, 
> +                int io_mode, int *error_code);
> +
> +static void ADIOI_LUSTRE_IOContig(ADIO_File fd, void *buf, int count, 
> +                   MPI_Datatype datatype, int file_ptr_type,
> +                ADIO_Offset offset, ADIO_Status *status, 
> +                int io_mode, int *error_code)
> +{
> +    int err=-1, datatype_size, len;

I don't think we need the declaration of ADIOI_LUSTRE_IOContig() immediately
before the function declaration.

> diff -ruNp romio-orig/adio/ad_lustre/ad_lustre_wrcoll.c 
> romio-ornl/adio/ad_lustre/ad_lustre_wrcoll.c
> --- romio-orig/adio/ad_lustre/ad_lustre_wrcoll.c      1969-12-31 
> 19:00:00.000000000 -0500
> +++ romio-ornl/adio/ad_lustre/ad_lustre_wrcoll.c      2007-07-14 
> 08:33:18.000000000 -0400
> @@ -0,0 +1,18 @@
> +void ADIOI_LUSTRE_WriteStridedColl(ADIO_File fd, void *buf, int count,
> +                       MPI_Datatype datatype, int file_ptr_type,
> +                       ADIO_Offset offset, ADIO_Status *status, int
> +                       *error_code)
> +{
> +    ADIOI_GEN_WriteStridedColl(fd, buf, count, datatype, file_ptr_type,
> +                           offset, status, error_code);
> +}

Is there a benefit to have such wrapper functions, or should we just
reference ADIOI_GEN_WriteStridedColl() directly in the method
table ADIO_LUSTRE_operations?

> diff -ruNp romio-orig/adio/include/adioi_fs_proto.h 
> romio-ornl/adio/include/adioi_fs_proto.h
> --- romio-orig/adio/include/adioi_fs_proto.h  2007-07-13 08:59:56.000064763 
> -0400
> +++ romio-ornl/adio/include/adioi_fs_proto.h  2007-07-13 20:52:01.000000000 
> -0400
> @@ -49,6 +49,32 @@ extern struct ADIOI_Fns_struct ADIO_SFS_
>  /* prototypes are in adio/ad_sfs/ad_sfs.h */
>  #endif
>  
> +#ifdef ROMIO_LUSTRE
> +extern struct ADIOI_Fns_struct ADIO_LUSTRE_operations;
> +
> +void ADIOI_LUSTRE_Open(ADIO_File fd, int *error_code);
> +void ADIOI_LUSTRE_Close(ADIO_File fd, int *error_code);
> +void ADIOI_LUSTRE_ReadContig(ADIO_File fd, void *buf, int count, 
> +                      MPI_Datatype datatype, int file_ptr_type,
> +                     ADIO_Offset offset, ADIO_Status *status, int
> +                  *error_code);
> +void ADIOI_LUSTRE_WriteContig(ADIO_File fd, void *buf, int count, 
> +                      MPI_Datatype datatype, int file_ptr_type,
> +                      ADIO_Offset offset, ADIO_Status *status, int
> +                   *error_code);   
> +void ADIOI_LUSTRE_WriteStridedColl(ADIO_File fd, void *buf, int count,
> +                    MPI_Datatype datatype, int file_ptr_type,
> +                    ADIO_Offset offset, ADIO_Status *status, int
> +                    *error_code);
> +void ADIOI_LUSTRE_ReadStridedColl(ADIO_File fd, void *buf, int count,
> +                    MPI_Datatype datatype, int file_ptr_type,
> +                    ADIO_Offset offset, ADIO_Status *status, int
> +                    *error_code);
> +void ADIOI_LUSTRE_Fcntl(ADIO_File fd, int flag, ADIO_Fcntl_t *fcntl_struct,
> +                    int *error_code);
> +void ADIOI_LUSTRE_SetInfo(ADIO_File fd, MPI_Info users_info, int 
> *error_code);
> +#endif

It would be cleaner to put these declarations into ad_lustre.h (which is
included in all the lustre .c files so the prototypes can be verified) and
then include that header here directly.  Having prototypes that are not
checked against the actual functions can allow bugs to be introduced silently.


Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

_______________________________________________
Lustre-discuss mailing list
[email protected]
https://mail.clusterfs.com/mailman/listinfo/lustre-discuss

Reply via email to