| Message ID | alpine.DEB.2.02.1511020751450.2096@localhost6.localdomain6 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Nov 02 2015 15:55, Julia Lawall wrote: > Move constant to the right of binary operators. > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > CC: Takashi Sakamoto <o-takashi@sakamocchi.jp> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > --- > > Depends on personal taste, but the modified version does look a little > nicer to me, since one can better see the relationship between the fdf > and sfc fields. > > amdtp-am824.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/sound/firewire/amdtp-am824.c > +++ b/sound/firewire/amdtp-am824.c > @@ -36,7 +36,7 @@ int amdtp_am824_set_parameters(struct am > if (err < 0) > return err; > > - s->fdf = AMDTP_FDF_AM824 | s->sfc; > + s->fdf = s->sfc | AMDTP_FDF_AM824; > > /* > * In IEC 61883-6, one data block represents one event. In ALSA, one Could I request explainations about the advantage of this modification? I cannot imagine good reasons for this subtle changes... Regards Takashi Sakamoto
On Mon, 2 Nov 2015, Takashi Sakamoto wrote: > Hi, > > On Nov 02 2015 15:55, Julia Lawall wrote: > > Move constant to the right of binary operators. > > > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > > > CC: Takashi Sakamoto <o-takashi@sakamocchi.jp> > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > > --- > > > > Depends on personal taste, but the modified version does look a little > > nicer to me, since one can better see the relationship between the fdf > > and sfc fields. > > > > amdtp-am824.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- a/sound/firewire/amdtp-am824.c > > +++ b/sound/firewire/amdtp-am824.c > > @@ -36,7 +36,7 @@ int amdtp_am824_set_parameters(struct am > > if (err < 0) > > return err; > > > > - s->fdf = AMDTP_FDF_AM824 | s->sfc; > > + s->fdf = s->sfc | AMDTP_FDF_AM824; > > > > /* > > * In IEC 61883-6, one data block represents one event. In ALSA, one > > Could I request explainations about the advantage of this modification? I > cannot imagine good reasons for this subtle changes... I put the reason under the --- (better see the relationship between fdf and sfc). But if you think the code makes more sense as it is, just ignore the patch. julia
Hi Julia, On Nov 02 2015 16:20, Julia Lawall wrote: > On Mon, 2 Nov 2015, Takashi Sakamoto wrote: > >> Hi, >> >> On Nov 02 2015 15:55, Julia Lawall wrote: >>> Move constant to the right of binary operators. >>> >>> Generated by: scripts/coccinelle/misc/compare_const_fl.cocci >>> >>> CC: Takashi Sakamoto <o-takashi@sakamocchi.jp> >>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> >>> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> >>> --- >>> >>> Depends on personal taste, but the modified version does look a little >>> nicer to me, since one can better see the relationship between the fdf >>> and sfc fields. >>> >>> amdtp-am824.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> --- a/sound/firewire/amdtp-am824.c >>> +++ b/sound/firewire/amdtp-am824.c >>> @@ -36,7 +36,7 @@ int amdtp_am824_set_parameters(struct am >>> if (err < 0) >>> return err; >>> >>> - s->fdf = AMDTP_FDF_AM824 | s->sfc; >>> + s->fdf = s->sfc | AMDTP_FDF_AM824; >>> >>> /* >>> * In IEC 61883-6, one data block represents one event. In ALSA, one >> >> Could I request explainations about the advantage of this modification? I >> cannot imagine good reasons for this subtle changes... > > I put the reason under the --- (better see the relationship between fdf > and sfc). But if you think the code makes more sense as it is, just > ignore the patch. Mmm. I'm sorry but I'm not wiling to add 'Reviewed-by' tag to this patch because the reason is non-functional and personal. Thanks Takashi Sakamoto
On Mon, 2 Nov 2015, Takashi Sakamoto wrote: > Hi Julia, > > On Nov 02 2015 16:20, Julia Lawall wrote: > > On Mon, 2 Nov 2015, Takashi Sakamoto wrote: > > > > > Hi, > > > > > > On Nov 02 2015 15:55, Julia Lawall wrote: > > > > Move constant to the right of binary operators. > > > > > > > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > > > > > > > CC: Takashi Sakamoto <o-takashi@sakamocchi.jp> > > > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > > > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > > > > --- > > > > > > > > Depends on personal taste, but the modified version does look a little > > > > nicer to me, since one can better see the relationship between the fdf > > > > and sfc fields. > > > > > > > > amdtp-am824.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > --- a/sound/firewire/amdtp-am824.c > > > > +++ b/sound/firewire/amdtp-am824.c > > > > @@ -36,7 +36,7 @@ int amdtp_am824_set_parameters(struct am > > > > if (err < 0) > > > > return err; > > > > > > > > - s->fdf = AMDTP_FDF_AM824 | s->sfc; > > > > + s->fdf = s->sfc | AMDTP_FDF_AM824; > > > > > > > > /* > > > > * In IEC 61883-6, one data block represents one event. In > > > > ALSA, one > > > > > > Could I request explainations about the advantage of this modification? I > > > cannot imagine good reasons for this subtle changes... > > > > I put the reason under the --- (better see the relationship between fdf > > and sfc). But if you think the code makes more sense as it is, just > > ignore the patch. > > Mmm. I'm sorry but I'm not wiling to add 'Reviewed-by' tag to this patch > because the reason is non-functional and personal. OK, no problem. julia
--- a/sound/firewire/amdtp-am824.c +++ b/sound/firewire/amdtp-am824.c @@ -36,7 +36,7 @@ int amdtp_am824_set_parameters(struct am if (err < 0) return err; - s->fdf = AMDTP_FDF_AM824 | s->sfc; + s->fdf = s->sfc | AMDTP_FDF_AM824; /* * In IEC 61883-6, one data block represents one event. In ALSA, one