On Fri, 2005-07-08 at 12:49, Miklos Szeredi wrote:
> > The reason why I implemented that way, is to less confuse the user and
> > provide more flexibility. With my implementation, we have the ability
> > to share any part of the tree, without bothering if it is a mountpoint
> > or not. The side
On Fri, 2005-07-08 at 12:49, Miklos Szeredi wrote:
The reason why I implemented that way, is to less confuse the user and
provide more flexibility. With my implementation, we have the ability
to share any part of the tree, without bothering if it is a mountpoint
or not. The side effect of
Hi,
On Mon, 11 Jul 2005, Horst von Brand wrote:
> > I don't generally disagree with that, I just think that defines are not
> > part of that list.
>
> Covered in "bad coding style" and "hard to read code", at least.
Somehow I missed the last lkml debate about where simple defines where a
Roman Zippel <[EMAIL PROTECTED]> wrote:
> On Sun, 10 Jul 2005, Pekka Enberg wrote:
[...]
> > Would you please be so kind to define your criteria for things that
> > "need fixing" so we could see if can reach some sort of an agreement on
> > this. My list is roughly as follows:
> >
> > -
Vojtech Pavlik <[EMAIL PROTECTED]> wrote:
> On Sun, Jul 10, 2005 at 09:21:42PM +0300, Pekka Enberg wrote:
> > Hmm. So we disagree on that issue as well. I think the point of review
> > is to improve code and help others conform with the existing coding
> > style which is why I find it strange that
Hi Roman,
Roman Zippel writes:
I don't generally disagree with that, I just think that defines are not
part of that list.
They're in Documentation/CodingStyle (see Chapter 11).
Roman Zippel writes:
Look, it's great that you do reviews, but please keep in mind it's the
author who has to
Hi Roman,
Roman Zippel writes:
I don't generally disagree with that, I just think that defines are not
part of that list.
They're in Documentation/CodingStyle (see Chapter 11).
Roman Zippel writes:
Look, it's great that you do reviews, but please keep in mind it's the
author who has to
Vojtech Pavlik [EMAIL PROTECTED] wrote:
On Sun, Jul 10, 2005 at 09:21:42PM +0300, Pekka Enberg wrote:
Hmm. So we disagree on that issue as well. I think the point of review
is to improve code and help others conform with the existing coding
style which is why I find it strange that you're
Roman Zippel [EMAIL PROTECTED] wrote:
On Sun, 10 Jul 2005, Pekka Enberg wrote:
[...]
Would you please be so kind to define your criteria for things that
need fixing so we could see if can reach some sort of an agreement on
this. My list is roughly as follows:
- Erroneous use of
Hi,
On Mon, 11 Jul 2005, Horst von Brand wrote:
I don't generally disagree with that, I just think that defines are not
part of that list.
Covered in bad coding style and hard to read code, at least.
Somehow I missed the last lkml debate about where simple defines where a
problem.
Hi!
> >>enums in C are (de?)promoted to integral types under most conditions, so
> >>the type-checking is useless.
> >
> >
> >It's a warning in gcc afaik and spare should complain as well.
>
> Check again.
Check sparse with -Wbitwise and enum properly marked as bitwise...
On Sun, Jul 10, 2005 at 09:21:42PM +0300, Pekka Enberg wrote:
> Hmm. So we disagree on that issue as well. I think the point of review
> is to improve code and help others conform with the existing coding
> style which is why I find it strange that you're suggesting me to limit
> my comments to a
Hi,
On Sun, 10 Jul 2005, Pekka Enberg wrote:
> > The point of a review is to comment on things that _need_ fixing. Less
> > experienced hackers take this a requirement for their drivers to be
> > included.
>
> Hmm. So we disagree on that issue as well. I think the point of review
> is to
On Sun, 10 Jul 2005 21:21:42 +0300 Pekka Enberg wrote:
| Hi Roman,
|
| At some point in time, I wrote:
| > > Roman, it is not as if I get to decide for the patch submitters. I
| > > comment on any issues _I_ have with the patch and the authors fix
| > > whatever they want (or what the
Hi Roman,
At some point in time, I wrote:
> > Roman, it is not as if I get to decide for the patch submitters. I
> > comment on any issues _I_ have with the patch and the authors fix
> > whatever they want (or what the maintainers ask for).
On Fri, 2005-07-08 at 21:59 +0200, Roman Zippel wrote:
On Friday 08 July 2005 19:57, Roman Zippel wrote:
> Hi,
>
> On Fri, 8 Jul 2005, Bryan Henderson wrote:
>
> > I wasn't aware anyone preferred defines to enums for declaring enumerated
> > data types.
>
> If it's really enumerated data types, that's fine, but this example was
> about bitfield
On Friday 08 July 2005 19:57, Roman Zippel wrote:
Hi,
On Fri, 8 Jul 2005, Bryan Henderson wrote:
I wasn't aware anyone preferred defines to enums for declaring enumerated
data types.
If it's really enumerated data types, that's fine, but this example was
about bitfield masks.
Hi Roman,
At some point in time, I wrote:
Roman, it is not as if I get to decide for the patch submitters. I
comment on any issues _I_ have with the patch and the authors fix
whatever they want (or what the maintainers ask for).
On Fri, 2005-07-08 at 21:59 +0200, Roman Zippel wrote:
The
On Sun, 10 Jul 2005 21:21:42 +0300 Pekka Enberg wrote:
| Hi Roman,
|
| At some point in time, I wrote:
| Roman, it is not as if I get to decide for the patch submitters. I
| comment on any issues _I_ have with the patch and the authors fix
| whatever they want (or what the maintainers ask
Hi,
On Sun, 10 Jul 2005, Pekka Enberg wrote:
The point of a review is to comment on things that _need_ fixing. Less
experienced hackers take this a requirement for their drivers to be
included.
Hmm. So we disagree on that issue as well. I think the point of review
is to improve code
On Sun, Jul 10, 2005 at 09:21:42PM +0300, Pekka Enberg wrote:
Hmm. So we disagree on that issue as well. I think the point of review
is to improve code and help others conform with the existing coding
style which is why I find it strange that you're suggesting me to limit
my comments to a
Hi!
enums in C are (de?)promoted to integral types under most conditions, so
the type-checking is useless.
It's a warning in gcc afaik and spare should complain as well.
Check again.
Check sparse with -Wbitwise and enum properly marked as bitwise...
Hi,
On Fri, 2005-07-08 at 21:11 +0200, Roman Zippel wrote:
> So it basically comes down to personal preference, if the original uses
> defines and it works fine, I don't really see a good enough reason to
> change it to enums, so please leave the decision to author.
(And I don't see a good
>I don't see how the following is tortured:
>
>enum {
> PNODE_MEMBER_VFS = 0x01,
> PNODE_SLAVE_VFS = 0x02
>};
Only because it's using a facility that's supposed to be for enumerated
types for something that isn't. If it were a true enumerated type, the
codes for the
Wichert Akkerman wrote:
Previously Mike Waychison wrote:
enums in C are (de?)promoted to integral types under most conditions, so
the type-checking is useless.
It's a warning in gcc afaik and spare should complain as well.
Check again.
You must be thinking of another language.
Show me
Hi,
On Fri, 8 Jul 2005, Pekka Enberg wrote:
> On Fri, 2005-07-08 at 21:11 +0200, Roman Zippel wrote:
> > So it basically comes down to personal preference, if the original uses
> > defines and it works fine, I don't really see a good enough reason to
> > change it to enums, so please leave the
> The reason why I implemented that way, is to less confuse the user and
> provide more flexibility. With my implementation, we have the ability
> to share any part of the tree, without bothering if it is a mountpoint
> or not. The side effect of this operation is, it ends up creating
> a
On Fri, 2005-07-08 at 12:11, Roman Zippel wrote:
> Hi,
>
> On Fri, 8 Jul 2005, Pekka J Enberg wrote:
>
> > I don't see how the following is tortured:
> > enum {
> > PNODE_MEMBER_VFS = 0x01,
> > PNODE_SLAVE_VFS = 0x02
> > };
> > In fact, I think it is more natural. An almost
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
> I don't see how the following is tortured:
> enum {
> PNODE_MEMBER_VFS = 0x01,
> PNODE_SLAVE_VFS = 0x02
> };
> In fact, I think it is more natural. An almost identical example even appears
> in K
So it basically comes down to
Roman Zippel writes:
> If it's really enumerated data types, that's fine, but this example was
> about bitfield masks.
Bryan Henderson writes:
Ah. In that case, enum is a pretty tortured way to declare it, though it
does have the practical advantages over define that have been mentioned
Previously Mike Waychison wrote:
> enums in C are (de?)promoted to integral types under most conditions, so
> the type-checking is useless.
It's a warning in gcc afaik and spare should complain as well.
Wichert.
--
Wichert Akkerman <[EMAIL PROTECTED]>It is simple to make things.
Wichert Akkerman wrote:
Previously Bryan Henderson wrote:
Two advantages of the enum declaration that haven't been mentioned yet,
that help me significantly:
There is another one: with enums the compiler checks types for you.
enums in C are (de?)promoted to integral types under most
Previously Bryan Henderson wrote:
> Two advantages of the enum declaration that haven't been mentioned yet,
> that help me significantly:
There is another one: with enums the compiler checks types for you.
Wichert.
--
Wichert Akkerman <[EMAIL PROTECTED]>It is simple to make things.
On Fri, 2005-07-08 at 09:51, Miklos Szeredi wrote:
> > > > + * recursively change the type of the mountpoint.
> > > > + */
> > > > +static int do_change_type(struct nameidata *nd, int flag)
> > > > +{
> > > > + struct vfsmount *m, *mnt;
> > > > + struct vfspnode *old_pnode = NULL;
> >
>If it's really enumerated data types, that's fine, but this example was
>about bitfield masks.
Ah. In that case, enum is a pretty tortured way to declare it, though it
does have the practical advantages over define that have been mentioned
because the syntax is more rigorous.
The proper way
Hi,
On Fri, 8 Jul 2005, Bryan Henderson wrote:
> I wasn't aware anyone preferred defines to enums for declaring enumerated
> data types.
If it's really enumerated data types, that's fine, but this example was
about bitfield masks.
> Isn't the only argument for defines, "that's what I'm used
> > > + * recursively change the type of the mountpoint.
> > > + */
> > > +static int do_change_type(struct nameidata *nd, int flag)
> > > +{
> > > + struct vfsmount *m, *mnt;
> > > + struct vfspnode *old_pnode = NULL;
> > > + int err;
> > > +
> > > + if (!(flag & MS_SHARED) && !(flag &
I wasn't aware anyone preferred defines to enums for declaring enumerated
data types. The practical advantages of enums are slight, but as far as I
know, the practical advantages of defines are zero. Isn't the only
argument for defines, "that's what I'm used to."?
Two advantages of the enum
On Fri, 2005-07-08 at 04:17, Pekka Enberg wrote:
> On 7/8/05, Ram <[EMAIL PROTECTED]> wrote:
> > This patch adds the shared/private/slave support for VFS trees.
>
> Inlining the patches to email would be greatly appreciated. Here are
> some comments.
>
> > +int
> > +_do_make_mounted(struct
On Fri, 2005-07-08 at 07:32, Miklos Szeredi wrote:
> > This patch adds the shared/private/slave support for VFS trees.
>
> [...]
>
> > -struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry)
> > +struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry,
> >
On Fri, 2005-07-08 at 15:34 +0200, Roman Zippel wrote:
> Are the advantages big enough to actively discourage defines? It's nice
> that you do reviews, but please leave some room for personal preferences.
> If the code is correct and perfectly readable, it doesn't matter whether
> to use
> This patch adds the shared/private/slave support for VFS trees.
[...]
> -struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry)
> +struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry,
> struct dentry *root)
> {
How about changing it to inline and
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
> > You can't do that with defines?
>
> Sure you can but have you ever tried to figure out where a group of #define
> enumerations end?
Comments? Newlines?
> Enums are a natural language construct for grouping related
> constants so why not use
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
> Hey, I just review patches. I don't get to set requirements. There's a reason
> why enums are preferred though. They define a proper name for the constant.
Roman Zippel writes:
Who prefers that?
Well, me, at least. I can't speak for others.
On
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
> On Fri, 8 Jul 2005, Pekka Enberg wrote:
> > > > +#define PNODE_MEMBER_VFS 0x01
> > > > +#define PNODE_SLAVE_VFS 0x02
> > > > Enums, please.
>
> Roman Zippel writes:
> > Is this becoming a requirement now? I personally would rather leave that to
On Fri, 8 Jul 2005, Pekka Enberg wrote:
> > +#define PNODE_MEMBER_VFS 0x01
> > +#define PNODE_SLAVE_VFS 0x02
>
> Enums, please.
Roman Zippel writes:
Is this becoming a requirement now? I personally would rather leave that
to personal preference...
Hey, I just review patches. I don't get
Hi,
On Fri, 8 Jul 2005, Pekka Enberg wrote:
> > +#define PNODE_MEMBER_VFS 0x01
> > +#define PNODE_SLAVE_VFS 0x02
>
> Enums, please.
Is this becoming a requirement now? I personally would rather leave that
to personal preference...
bye, Roman
-
To unsubscribe from this list: send the line
On 7/8/05, Ram <[EMAIL PROTECTED]> wrote:
> This patch adds the shared/private/slave support for VFS trees.
Inlining the patches to email would be greatly appreciated. Here are
some comments.
> +int
> +_do_make_mounted(struct nameidata *nd, struct vfsmount **mnt)
Use two underscores to follow
This patch adds the shared/private/slave support for VFS trees.
RP
This patch adds the shared/private/slave support for VFS trees.
Signed by Ram Pai ([EMAIL PROTECTED])
fs/Makefile|2
fs/dcache.c|2
fs/namei.c |4
fs/namespace.c |
This patch adds the shared/private/slave support for VFS trees.
RP
This patch adds the shared/private/slave support for VFS trees.
Signed by Ram Pai ([EMAIL PROTECTED])
fs/Makefile|2
fs/dcache.c|2
fs/namei.c |4
fs/namespace.c |
On 7/8/05, Ram [EMAIL PROTECTED] wrote:
This patch adds the shared/private/slave support for VFS trees.
Inlining the patches to email would be greatly appreciated. Here are
some comments.
+int
+_do_make_mounted(struct nameidata *nd, struct vfsmount **mnt)
Use two underscores to follow naming
Hi,
On Fri, 8 Jul 2005, Pekka Enberg wrote:
+#define PNODE_MEMBER_VFS 0x01
+#define PNODE_SLAVE_VFS 0x02
Enums, please.
Is this becoming a requirement now? I personally would rather leave that
to personal preference...
bye, Roman
-
To unsubscribe from this list: send the line
On Fri, 8 Jul 2005, Pekka Enberg wrote:
+#define PNODE_MEMBER_VFS 0x01
+#define PNODE_SLAVE_VFS 0x02
Enums, please.
Roman Zippel writes:
Is this becoming a requirement now? I personally would rather leave that
to personal preference...
Hey, I just review patches. I don't get to
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
On Fri, 8 Jul 2005, Pekka Enberg wrote:
+#define PNODE_MEMBER_VFS 0x01
+#define PNODE_SLAVE_VFS 0x02
Enums, please.
Roman Zippel writes:
Is this becoming a requirement now? I personally would rather leave that to
personal
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
Hey, I just review patches. I don't get to set requirements. There's a reason
why enums are preferred though. They define a proper name for the constant.
Roman Zippel writes:
Who prefers that?
Well, me, at least. I can't speak for others.
On
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
You can't do that with defines?
Sure you can but have you ever tried to figure out where a group of #define
enumerations end?
Comments? Newlines?
Enums are a natural language construct for grouping related
constants so why not use it?
So
This patch adds the shared/private/slave support for VFS trees.
[...]
-struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry)
+struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry,
struct dentry *root)
{
How about changing it to inline and calling it
On Fri, 2005-07-08 at 15:34 +0200, Roman Zippel wrote:
Are the advantages big enough to actively discourage defines? It's nice
that you do reviews, but please leave some room for personal preferences.
If the code is correct and perfectly readable, it doesn't matter whether
to use defines or
On Fri, 2005-07-08 at 07:32, Miklos Szeredi wrote:
This patch adds the shared/private/slave support for VFS trees.
[...]
-struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry)
+struct vfsmount *lookup_mnt(struct vfsmount *mnt, struct dentry *dentry,
struct dentry
On Fri, 2005-07-08 at 04:17, Pekka Enberg wrote:
On 7/8/05, Ram [EMAIL PROTECTED] wrote:
This patch adds the shared/private/slave support for VFS trees.
Inlining the patches to email would be greatly appreciated. Here are
some comments.
+int
+_do_make_mounted(struct nameidata *nd,
I wasn't aware anyone preferred defines to enums for declaring enumerated
data types. The practical advantages of enums are slight, but as far as I
know, the practical advantages of defines are zero. Isn't the only
argument for defines, that's what I'm used to.?
Two advantages of the enum
+ * recursively change the type of the mountpoint.
+ */
+static int do_change_type(struct nameidata *nd, int flag)
+{
+ struct vfsmount *m, *mnt;
+ struct vfspnode *old_pnode = NULL;
+ int err;
+
+ if (!(flag MS_SHARED) !(flag MS_PRIVATE)
+ !(flag
Hi,
On Fri, 8 Jul 2005, Bryan Henderson wrote:
I wasn't aware anyone preferred defines to enums for declaring enumerated
data types.
If it's really enumerated data types, that's fine, but this example was
about bitfield masks.
Isn't the only argument for defines, that's what I'm used to.?
If it's really enumerated data types, that's fine, but this example was
about bitfield masks.
Ah. In that case, enum is a pretty tortured way to declare it, though it
does have the practical advantages over define that have been mentioned
because the syntax is more rigorous.
The proper way
On Fri, 2005-07-08 at 09:51, Miklos Szeredi wrote:
+ * recursively change the type of the mountpoint.
+ */
+static int do_change_type(struct nameidata *nd, int flag)
+{
+ struct vfsmount *m, *mnt;
+ struct vfspnode *old_pnode = NULL;
+ int err;
+
Previously Bryan Henderson wrote:
Two advantages of the enum declaration that haven't been mentioned yet,
that help me significantly:
There is another one: with enums the compiler checks types for you.
Wichert.
--
Wichert Akkerman [EMAIL PROTECTED]It is simple to make things.
Wichert Akkerman wrote:
Previously Bryan Henderson wrote:
Two advantages of the enum declaration that haven't been mentioned yet,
that help me significantly:
There is another one: with enums the compiler checks types for you.
enums in C are (de?)promoted to integral types under most
Previously Mike Waychison wrote:
enums in C are (de?)promoted to integral types under most conditions, so
the type-checking is useless.
It's a warning in gcc afaik and spare should complain as well.
Wichert.
--
Wichert Akkerman [EMAIL PROTECTED]It is simple to make things.
Roman Zippel writes:
If it's really enumerated data types, that's fine, but this example was
about bitfield masks.
Bryan Henderson writes:
Ah. In that case, enum is a pretty tortured way to declare it, though it
does have the practical advantages over define that have been mentioned
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
I don't see how the following is tortured:
enum {
PNODE_MEMBER_VFS = 0x01,
PNODE_SLAVE_VFS = 0x02
};
In fact, I think it is more natural. An almost identical example even appears
in KR.
So it basically comes down to personal
On Fri, 2005-07-08 at 12:11, Roman Zippel wrote:
Hi,
On Fri, 8 Jul 2005, Pekka J Enberg wrote:
I don't see how the following is tortured:
enum {
PNODE_MEMBER_VFS = 0x01,
PNODE_SLAVE_VFS = 0x02
};
In fact, I think it is more natural. An almost identical example
The reason why I implemented that way, is to less confuse the user and
provide more flexibility. With my implementation, we have the ability
to share any part of the tree, without bothering if it is a mountpoint
or not. The side effect of this operation is, it ends up creating
a vfsmount if
Hi,
On Fri, 8 Jul 2005, Pekka Enberg wrote:
On Fri, 2005-07-08 at 21:11 +0200, Roman Zippel wrote:
So it basically comes down to personal preference, if the original uses
defines and it works fine, I don't really see a good enough reason to
change it to enums, so please leave the
Wichert Akkerman wrote:
Previously Mike Waychison wrote:
enums in C are (de?)promoted to integral types under most conditions, so
the type-checking is useless.
It's a warning in gcc afaik and spare should complain as well.
Check again.
You must be thinking of another language.
Show me
I don't see how the following is tortured:
enum {
PNODE_MEMBER_VFS = 0x01,
PNODE_SLAVE_VFS = 0x02
};
Only because it's using a facility that's supposed to be for enumerated
types for something that isn't. If it were a true enumerated type, the
codes for the enumerations
Hi,
On Fri, 2005-07-08 at 21:11 +0200, Roman Zippel wrote:
So it basically comes down to personal preference, if the original uses
defines and it works fine, I don't really see a good enough reason to
change it to enums, so please leave the decision to author.
(And I don't see a good enough
76 matches
Mail list logo
