On Mon, Jul 24, 2017 at 08:19:00AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Jul 24, 2017 at 07:19:45AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Sun, Jul 23, 2017 at 10:37:05AM +0100, Frediano Ziglio wrote:
> > > > > The "throw" statement alone are used to throw an already existing
> > > > > exception however in log_version they were meant to raise an exception.
> > > > > This would have caused a termination of the program.
> > > > >
> > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > > ---
> > > > > common/vdlog.cpp | 17 +++++++++--------
> > > > > 1 file changed, 9 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/common/vdlog.cpp b/common/vdlog.cpp
> > > > > index c983127..0a4c816 100644
> > > > > --- a/common/vdlog.cpp
> > > > > +++ b/common/vdlog.cpp
> > > > > @@ -79,33 +79,34 @@ void VDLog::printf(const char* format, ...)
> > > > >
> > > > > void log_version()
> > > > > {
> > > > > - DWORD handle;
> > > > > - TCHAR module_fname[MAX_PATH];
> > > > > + if (![]() -> bool {
> > > >
> > > > I don't think a lambda is really useful/required here..
> > > >
> > >
> > > Nothing is required. Is fixing a bug so the code is surely useful.
> > >
> > > Counterproposal?
> >
> > s/throw/throw std::runtime_error("something"), or have an actual
> > function instead of the lambda.
> >
> > >
> > > Some really radical can be
> > >
> > > void log_version()
> > > {
> > > vd_printf("%u.%u.%u.%u", RC_PRODUCTVERSION);
> >
> > Yup, or that, but I'd use WINDOWS_PRODUCTVERSION
> >
> > Christophe
> >
>
> Already sent the patch.
>
> RC_PRODUCTVERSION was already defined, I just reused that.
> I agree "RC_PRODUCTVERSION" is not really understandable at the first
> sight (why something related to resources?) but "WINDOWS" looks more
> related to the OS. Considering that old code get the information from
> resources that change and constant make sense. Maybe a comment for who
> is going to read the code like
>
> // print same version as resource one
> vd_printf("%u.%u.%u.%u", RC_PRODUCTVERSION);
Most important thing to mention in a comment is that RC_PRODUCTVERSION is defined as
$major, $minor, $microdigit, 0.
I checked that we get warnings from the compiler if we pass too many/not
enough args.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel