[PATCH v2] staging: lustre: Replace typedef with struct

[PATCH v2] staging: lustre: Replace typedef with struct

[Gargi Sharma]

As per the Linux kernel coding style guidelines, using typedef for a
structure type is not recommended. Hence, occurences of typedefs have
been removed. To find the occurences of the structures grep was used
and no uses were found.

Coccinelle script:

@r1@
type T;
@@

typedef struct { ... } T;

@script:python c1@
T2;
T << r1.T;
@@
coccinelle.T2=T;

@@
type r1.T;
identifier c1.T2;
@@
-typedef
struct
+ T2
{ ... }
-T
;

Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
Changes in v2:
	- Improved the commit message.
---
 drivers/staging/lustre/lustre/include/lustre_eacl.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_eacl.h b/drivers/staging/lustre/lustre/include/lustre_eacl.h
index 1e71a86..c198eda 100644
--- a/drivers/staging/lustre/lustre/include/lustre_eacl.h
+++ b/drivers/staging/lustre/lustre/include/lustre_eacl.h
@@ -49,17 +49,17 @@
 #include <linux/fs.h>
 #include <linux/posix_acl_xattr.h>
 
-typedef struct {
+struct ext_acl_xattr_entry {
 	__u16		   e_tag;
 	__u16		   e_perm;
 	__u32		   e_id;
 	__u32		   e_stat;
-} ext_acl_xattr_entry;
+};
 
-typedef struct {
-	__u32		   a_count;
-	ext_acl_xattr_entry     a_entries[0];
-} ext_acl_xattr_header;
+struct ext_acl_xattr_header {
+	__u32 a_count;
+	struct ext_acl_xattr_entry a_entries[0];
+};
 
 #define CFS_ACL_XATTR_SIZE(count, prefix) \
 	(sizeof(prefix ## _header) + (count) * sizeof(prefix ## _entry))
-- 
2.7.4

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Julia Lawall]

On Tue, 28 Feb 2017, Gargi Sharma wrote:

> As per the Linux kernel coding style guidelines, using typedef for a
> structure type is not recommended. Hence, occurences of typedefs have
> been removed. To find the occurences of the structures grep was used
> and no uses were found.

Actually, I'm not sure to understand.  No uses of the structures were
found at all?  If so, they should just be dropped.  Or are there uses, but
they already have the word "struct" in front of them?

julia

>
> Coccinelle script:
>
> @r1@
> type T;
> @@
>
> typedef struct { ... } T;
>
> @script:python c1@
> T2;
> T << r1.T;
> @@
> coccinelle.T2=T;
>
> @@
> type r1.T;
> identifier c1.T2;
> @@
> -typedef
> struct
> + T2
> { ... }
> -T
> ;
>
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> ---
> Changes in v2:
> 	- Improved the commit message.
> ---
>  drivers/staging/lustre/lustre/include/lustre_eacl.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_eacl.h b/drivers/staging/lustre/lustre/include/lustre_eacl.h
> index 1e71a86..c198eda 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_eacl.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_eacl.h
> @@ -49,17 +49,17 @@
>  #include <linux/fs.h>
>  #include <linux/posix_acl_xattr.h>
>
> -typedef struct {
> +struct ext_acl_xattr_entry {
>  	__u16		   e_tag;
>  	__u16		   e_perm;
>  	__u32		   e_id;
>  	__u32		   e_stat;
> -} ext_acl_xattr_entry;
> +};
>
> -typedef struct {
> -	__u32		   a_count;
> -	ext_acl_xattr_entry     a_entries[0];
> -} ext_acl_xattr_header;
> +struct ext_acl_xattr_header {
> +	__u32 a_count;
> +	struct ext_acl_xattr_entry a_entries[0];
> +};
>
>  #define CFS_ACL_XATTR_SIZE(count, prefix) \
>  	(sizeof(prefix ## _header) + (count) * sizeof(prefix ## _entry))
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1488265307-24641-1-git-send-email-gs051095%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 2972 bytes --]

On Tue, Feb 28, 2017 at 12:36 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>
> On Tue, 28 Feb 2017, Gargi Sharma wrote:
>
> > As per the Linux kernel coding style guidelines, using typedef for a
> > structure type is not recommended. Hence, occurences of typedefs have
> > been removed. To find the occurences of the structures grep was used
> > and no uses were found.
>
> Actually, I'm not sure to understand.  No uses of the structures were
> found at all?  If so, they should just be dropped.  Or are there uses, but
> they already have the word "struct" in front of them?

The first structure "ext_acl_xattr_entry" is used as one of the fields in
the
other structure "ext_acl_xattr_header". I could not find any uses of the
structure
"ext_acl_xattr_header" inside the staging directory. Should I drop these two
structures then?

gargi
>
> julia
>
> >
> > Coccinelle script:
> >
> > @r1@
> > type T;
> > @@
> >
> > typedef struct { ... } T;
> >
> > @script:python c1@
> > T2;
> > T << r1.T;
> > @@
> > coccinelle.T2=T;
> >
> > @@
> > type r1.T;
> > identifier c1.T2;
> > @@
> > -typedef
> > struct
> > + T2
> > { ... }
> > -T
> > ;
> >
> > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> > ---
> > Changes in v2:
> >       - Improved the commit message.
> > ---
> >  drivers/staging/lustre/lustre/include/lustre_eacl.h | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/include/lustre_eacl.h
b/drivers/staging/lustre/lustre/include/lustre_eacl.h
> > index 1e71a86..c198eda 100644
> > --- a/drivers/staging/lustre/lustre/include/lustre_eacl.h
> > +++ b/drivers/staging/lustre/lustre/include/lustre_eacl.h
> > @@ -49,17 +49,17 @@
> >  #include <linux/fs.h>
> >  #include <linux/posix_acl_xattr.h>
> >
> > -typedef struct {
> > +struct ext_acl_xattr_entry {
> >       __u16              e_tag;
> >       __u16              e_perm;
> >       __u32              e_id;
> >       __u32              e_stat;
> > -} ext_acl_xattr_entry;
> > +};
> >
> > -typedef struct {
> > -     __u32              a_count;
> > -     ext_acl_xattr_entry     a_entries[0];
> > -} ext_acl_xattr_header;
> > +struct ext_acl_xattr_header {
> > +     __u32 a_count;
> > +     struct ext_acl_xattr_entry a_entries[0];
> > +};
> >
> >  #define CFS_ACL_XATTR_SIZE(count, prefix) \
> >       (sizeof(prefix ## _header) + (count) * sizeof(prefix ## _entry))
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google
Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/
msgid/outreachy-kernel/1488265307-24641-1-git-send-
email-gs051095%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

[-- Attachment #2: Type: text/html, Size: 4775 bytes --]

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Greg KH]

On Tue, Feb 28, 2017 at 01:20:59PM +0530, Gargi Sharma wrote:
> On Tue, Feb 28, 2017 at 12:36 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> >
> > On Tue, 28 Feb 2017, Gargi Sharma wrote:
> >
> > > As per the Linux kernel coding style guidelines, using typedef for a
> > > structure type is not recommended. Hence, occurences of typedefs have
> > > been removed. To find the occurences of the structures grep was used
> > > and no uses were found.
> >
> > Actually, I'm not sure to understand.� No uses of the structures were
> > found at all?� If so, they should just be dropped.� Or are there uses, but
> > they already have the word "struct" in front of them?
> 
> The first structure "ext_acl_xattr_entry" is used as one of the fields in the�
> other structure "ext_acl_xattr_header". I could not find any uses of the
> structure
> "ext_acl_xattr_header" inside the staging directory. Should I drop these two
> structures then?

I think so, we shouldn't keep around structures that are not actually
used.

Unless the Lustre maintainers are hiding something somewhere else that
uses these?

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Dilger, Andreas]

On Feb 28, 2017, at 01:18, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> On Tue, Feb 28, 2017 at 01:20:59PM +0530, Gargi Sharma wrote:
>> On Tue, Feb 28, 2017 at 12:36 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>> 
>>> 
>>> 
>>> On Tue, 28 Feb 2017, Gargi Sharma wrote:
>>> 
>>>> As per the Linux kernel coding style guidelines, using typedef for a
>>>> structure type is not recommended. Hence, occurences of typedefs have
>>>> been removed. To find the occurences of the structures grep was used
>>>> and no uses were found.
>>> 
>>> Actually, I'm not sure to understand.  No uses of the structures were
>>> found at all?  If so, they should just be dropped.  Or are there uses, but
>>> they already have the word "struct" in front of them?
>> 
>> The first structure "ext_acl_xattr_entry" is used as one of the fields in the 
>> other structure "ext_acl_xattr_header". I could not find any uses of the
>> structure
>> "ext_acl_xattr_header" inside the staging directory. Should I drop these two
>> structures then?
> 
> I think so, we shouldn't keep around structures that are not actually
> used.
> 
> Unless the Lustre maintainers are hiding something somewhere else that
> uses these?

Looks like this is left over from 341f1f0affed1c24712f37c95bb654b3b33ab2c6
"staging: lustre: remove remote client support".  The ext_acl_xattr_{entry,header}
structs were accessed from CFS_ACL_XATTR_SIZE() and CFS_ACL_XATTR_COUNT() macros,
along with posix_acl_xattr_{entry,header} but it looks like all of that was deleted
(and good riddance).  The CFS_ACL_XATTR_* macros should be removed also.

That leaves the lustre_eacl.h header empty, so it could just be deleted entirely,
and removed from llite/llite_internal.h (where posix_acl_xattr.h is already included)
and llite/xattr.h (which includes llite_internal.h).

Cheers, Andreas

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On Thu, Mar 2, 2017 at 4:17 PM, Dilger, Andreas <andreas.dilger@intel.com>
wrote:
>
> On Feb 28, 2017, at 01:18, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Feb 28, 2017 at 01:20:59PM +0530, Gargi Sharma wrote:
> >> On Tue, Feb 28, 2017 at 12:36 PM, Julia Lawall <julia.lawall@lip6.fr>
wrote:
> >>>
> >>>
> >>>
> >>> On Tue, 28 Feb 2017, Gargi Sharma wrote:
> >>>
> >>>> As per the Linux kernel coding style guidelines, using typedef for a
> >>>> structure type is not recommended. Hence, occurences of typedefs have
> >>>> been removed. To find the occurences of the structures grep was used
> >>>> and no uses were found.
> >>>
> >>> Actually, I'm not sure to understand.  No uses of the structures were
> >>> found at all?  If so, they should just be dropped.  Or are there
uses, but
> >>> they already have the word "struct" in front of them?
> >>
> >> The first structure "ext_acl_xattr_entry" is used as one of the fields
in the
> >> other structure "ext_acl_xattr_header". I could not find any uses of
the
> >> structure
> >> "ext_acl_xattr_header" inside the staging directory. Should I drop
these two
> >> structures then?
> >
> > I think so, we shouldn't keep around structures that are not actually
> > used.
> >
> > Unless the Lustre maintainers are hiding something somewhere else that
> > uses these?
>
> Looks like this is left over from 341f1f0affed1c24712f37c95bb654b3b33ab2c6
> "staging: lustre: remove remote client support".  The
ext_acl_xattr_{entry,header}
> structs were accessed from CFS_ACL_XATTR_SIZE() and CFS_ACL_XATTR_COUNT()
macros,
> along with posix_acl_xattr_{entry,header} but it looks like all of that
was deleted
> (and good riddance).  The CFS_ACL_XATTR_* macros should be removed also.
>
Hi,

>
> That leaves the lustre_eacl.h header empty, so it could just be deleted
entirely,
> and removed from llite/llite_internal.h (where posix_acl_xattr.h is
already included)
> and llite/xattr.h (which includes llite_internal.h).

Okay, that makes sense. So should I send a v3 where I delete the header
file and places it
has been referred, or an entirely new patch?

thanks,
gargi

[-- Attachment #2: Type: text/html, Size: 2979 bytes --]

Re: [Outreachy kernel] [PATCH v2] staging: lustre: Replace typedef with struct

[Dilger, Andreas]

On Mar 2, 2017, at 10:21, Gargi Sharma <gs051095@gmail.com> wrote:
> 
> On Thu, Mar 2, 2017 at 4:17 PM, Dilger, Andreas <andreas.dilger@intel.com> wrote:
> >
> > On Feb 28, 2017, at 01:18, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Feb 28, 2017 at 01:20:59PM +0530, Gargi Sharma wrote:
> > >> On Tue, Feb 28, 2017 at 12:36 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On Tue, 28 Feb 2017, Gargi Sharma wrote:
> > >>>
> > >>>> As per the Linux kernel coding style guidelines, using typedef for a
> > >>>> structure type is not recommended. Hence, occurences of typedefs have
> > >>>> been removed. To find the occurences of the structures grep was used
> > >>>> and no uses were found.
> > >>>
> > >>> Actually, I'm not sure to understand.  No uses of the structures were
> > >>> found at all?  If so, they should just be dropped.  Or are there uses, but
> > >>> they already have the word "struct" in front of them?
> > >>
> > >> The first structure "ext_acl_xattr_entry" is used as one of the fields in the
> > >> other structure "ext_acl_xattr_header". I could not find any uses of the
> > >> structure
> > >> "ext_acl_xattr_header" inside the staging directory. Should I drop these two
> > >> structures then?
> > >
> > > I think so, we shouldn't keep around structures that are not actually
> > > used.
> > >
> > > Unless the Lustre maintainers are hiding something somewhere else that
> > > uses these?
> >
> > Looks like this is left over from 341f1f0affed1c24712f37c95bb654b3b33ab2c6
> > "staging: lustre: remove remote client support".  The ext_acl_xattr_{entry,header}
> > structs were accessed from CFS_ACL_XATTR_SIZE() and CFS_ACL_XATTR_COUNT() macros,
> > along with posix_acl_xattr_{entry,header} but it looks like all of that was deleted
> > (and good riddance).  The CFS_ACL_XATTR_* macros should be removed also.
> >
> Hi,
> 
> > 
> > That leaves the lustre_eacl.h header empty, so it could just be deleted entirely,
> > and removed from llite/llite_internal.h (where posix_acl_xattr.h is already included)
> > and llite/xattr.h (which includes llite_internal.h).
> 
> Okay, that makes sense. So should I send a v3 where I delete the header file and places it
> has been referred, or an entirely new patch?

If the whole header is being deleted, there isn't any point to have the earlier patch.
It makes the most sense to just have a single patch that deletes the unused header.

Cheers, Andreas
--
Andreas Dilger