Re: [Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
On Mon, Jun 12, 2017 at 11:21 AM Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > Reviewed-by: Marc-André Lureau > --- > common/quic.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/quic.c b/common/quic.c > index 5b00d65..59565ae 100644 > --- a/common/quic.c > +++ b/common/quic.c > @@ -173,13 +173,13 @@ struct Encoder { > }; > > /* target wait mask index */ > -static int wmimax = DEFwmimax; > +static const int wmimax = DEFwmimax; > > /* number of symbols to encode before increasing wait mask index */ > -static int wminext = DEFwminext; > +static const int wminext = DEFwminext; > > /* model evolution mode */ > -static int evol = DEFevol; > +static const int evol = DEFevol; > > /* bppmask[i] contains i ones as lsb-s */ > static const unsigned long int bppmask[33] = { > -- > 2.9.4 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel > -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
> > Hi > > - Original Message - > > > > > > - Original Message - > > > > Signed-off-by: Frediano Ziglio > > > > --- > > > > common/quic.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/common/quic.c b/common/quic.c > > > > index 5b00d65..59565ae 100644 > > > > --- a/common/quic.c > > > > +++ b/common/quic.c > > > > @@ -173,13 +173,13 @@ struct Encoder { > > > > }; > > > > > > > > /* target wait mask index */ > > > > -static int wmimax = DEFwmimax; > > > > +static const int wmimax = DEFwmimax; > > > > > > > > /* number of symbols to encode before increasing wait mask index */ > > > > -static int wminext = DEFwminext; > > > > +static const int wminext = DEFwminext; > > > > > > > > /* model evolution mode */ > > > > -static int evol = DEFevol; > > > > +static const int evol = DEFevol; > > > > > > > > /* bppmask[i] contains i ones as lsb-s */ > > > > static const unsigned long int bppmask[33] = { > > > > > > > > > Ah right, you fixed it here. I wonder if we should stick to a upper case > > > convention for const global values. And I wonder if it shouldn't be a > > > #define instead. > > > > > > > Usually for compatibility I use defines but I don't think is > > a big problem for the compilers we use. A static const should be > > a compile time constant (OT: a non static one is not so in C > > but so in C++). > > > > I am not thinking about compile-time optimization, but rather making clear > that the value can't be tweaked (even with a debugger) by using upper-case > macro convention. > > Frediano > > > A compile time constant and a define requires the same kind of tweaks from a debugger prospective. The only difference from a debugger is that with the define macro you don't see the mnemonic. As a developer if I read "static const" I assume is a constant (I don't really understand your confusion here). And on the usage occurrences (like "switch (evol)") the compiler won't allow me to change the constant. I think this code was written that way to allow to change (maybe even at runtime) these variable so it made sense. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
Hi - Original Message - > > > > - Original Message - > > > Signed-off-by: Frediano Ziglio > > > --- > > > common/quic.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/common/quic.c b/common/quic.c > > > index 5b00d65..59565ae 100644 > > > --- a/common/quic.c > > > +++ b/common/quic.c > > > @@ -173,13 +173,13 @@ struct Encoder { > > > }; > > > > > > /* target wait mask index */ > > > -static int wmimax = DEFwmimax; > > > +static const int wmimax = DEFwmimax; > > > > > > /* number of symbols to encode before increasing wait mask index */ > > > -static int wminext = DEFwminext; > > > +static const int wminext = DEFwminext; > > > > > > /* model evolution mode */ > > > -static int evol = DEFevol; > > > +static const int evol = DEFevol; > > > > > > /* bppmask[i] contains i ones as lsb-s */ > > > static const unsigned long int bppmask[33] = { > > > > > > Ah right, you fixed it here. I wonder if we should stick to a upper case > > convention for const global values. And I wonder if it shouldn't be a > > #define instead. > > > > Usually for compatibility I use defines but I don't think is > a big problem for the compilers we use. A static const should be > a compile time constant (OT: a non static one is not so in C > but so in C++). > I am not thinking about compile-time optimization, but rather making clear that the value can't be tweaked (even with a debugger) by using upper-case macro convention. > Frediano > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
> > - Original Message - > > Signed-off-by: Frediano Ziglio > > --- > > common/quic.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/common/quic.c b/common/quic.c > > index 5b00d65..59565ae 100644 > > --- a/common/quic.c > > +++ b/common/quic.c > > @@ -173,13 +173,13 @@ struct Encoder { > > }; > > > > /* target wait mask index */ > > -static int wmimax = DEFwmimax; > > +static const int wmimax = DEFwmimax; > > > > /* number of symbols to encode before increasing wait mask index */ > > -static int wminext = DEFwminext; > > +static const int wminext = DEFwminext; > > > > /* model evolution mode */ > > -static int evol = DEFevol; > > +static const int evol = DEFevol; > > > > /* bppmask[i] contains i ones as lsb-s */ > > static const unsigned long int bppmask[33] = { > > > Ah right, you fixed it here. I wonder if we should stick to a upper case > convention for const global values. And I wonder if it shouldn't be a > #define instead. > Usually for compatibility I use defines but I don't think is a big problem for the compilers we use. A static const should be a compile time constant (OT: a non static one is not so in C but so in C++). Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
- Original Message - > Signed-off-by: Frediano Ziglio > --- > common/quic.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/quic.c b/common/quic.c > index 5b00d65..59565ae 100644 > --- a/common/quic.c > +++ b/common/quic.c > @@ -173,13 +173,13 @@ struct Encoder { > }; > > /* target wait mask index */ > -static int wmimax = DEFwmimax; > +static const int wmimax = DEFwmimax; > > /* number of symbols to encode before increasing wait mask index */ > -static int wminext = DEFwminext; > +static const int wminext = DEFwminext; > > /* model evolution mode */ > -static int evol = DEFevol; > +static const int evol = DEFevol; > > /* bppmask[i] contains i ones as lsb-s */ > static const unsigned long int bppmask[33] = { Ah right, you fixed it here. I wonder if we should stick to a upper case convention for const global values. And I wonder if it shouldn't be a #define instead. > -- > 2.9.4 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 1/2] quic: constantify some variable
Signed-off-by: Frediano Ziglio --- common/quic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/quic.c b/common/quic.c index 5b00d65..59565ae 100644 --- a/common/quic.c +++ b/common/quic.c @@ -173,13 +173,13 @@ struct Encoder { }; /* target wait mask index */ -static int wmimax = DEFwmimax; +static const int wmimax = DEFwmimax; /* number of symbols to encode before increasing wait mask index */ -static int wminext = DEFwminext; +static const int wminext = DEFwminext; /* model evolution mode */ -static int evol = DEFevol; +static const int evol = DEFevol; /* bppmask[i] contains i ones as lsb-s */ static const unsigned long int bppmask[33] = { -- 2.9.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel