Re: giada - new release

[Jaromír Mikeš]

2018-01-17 17:19 GMT+01:00 James Cowgill :

> Hi,
>
> On 17/01/18 15:42, Jaromír Mikeš wrote:
> > ​Hi,
> >
> > giada unfortunately fail to build on some archs ... I already informed
> > upstream but not answer yet.
> > Can someone look if fixing this is rather trivial or complicated.
> >
> > https://buildd.debian.org/status/package.php?p=giada
>
> The entire function for reference:
> > std::string gu_format(const char* format, ...)
> > {
> >   va_list args;
> >
> >   /* Compute the size of the new expanded string (i.e. with
> replacement taken
> >   into account). */
> >
> >   size_t size = vsnprintf(nullptr, 0, format, args);
> >
> >   /* Create a new temporary char array to hold the new expanded
> string. */
> >
> >   std::unique_ptr tmp(new char[size]);
> >
> >   /* Fill the temporary string with the formatted data. */
> >
> >   va_start(args, format);
> >   vsprintf(tmp.get(), format, args);
> >   va_end(args);
> >
> >   return string(tmp.get(), tmp.get() + size - 1);
> > }
>
> This line (the one the error complains about) reads the uninitialized
> args and invokes undefined behavior:
> > size_t size = vsnprintf(nullptr, 0, format, args);
>
> It needs to be surrounded in va_start, va_end block.
>
> The second subtle error is that vsnprintf returns the size _excluding
> the null byte_. This will cause the vsprintf call to overflow the buffer
> by 1 byte.
>
> This might work (untested):
>  va_start(args, format);
>  size_t size = vsnprintf(nullptr, 0, format, args) + 1;
>  va_end(args);
>
> Some alternative implementations. The varardic template solution (the
> third one) is similar to this code (and the one I like the most):
> https://stackoverflow.com/questions/2342162/stdstring-
> formatting-like-sprintf


​Thank you James! Uploaded! Let's see.

mira
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Re: giada - new release

[James Cowgill]

Hi,

On 17/01/18 15:42, Jaromír Mikeš wrote:
> ​Hi,
> 
> giada unfortunately fail to build on some archs ... I already informed
> upstream but not answer yet.
> Can someone look if fixing this is rather trivial or complicated.
> 
> https://buildd.debian.org/status/package.php?p=giada

The entire function for reference:
> std::string gu_format(const char* format, ...)
> {
>   va_list args;
> 
>   /* Compute the size of the new expanded string (i.e. with replacement 
> taken
>   into account). */
> 
>   size_t size = vsnprintf(nullptr, 0, format, args);
> 
>   /* Create a new temporary char array to hold the new expanded string. */
> 
>   std::unique_ptr tmp(new char[size]);
> 
>   /* Fill the temporary string with the formatted data. */
> 
>   va_start(args, format);
>   vsprintf(tmp.get(), format, args);
>   va_end(args);
>   
>   return string(tmp.get(), tmp.get() + size - 1); 
> }

This line (the one the error complains about) reads the uninitialized
args and invokes undefined behavior:
> size_t size = vsnprintf(nullptr, 0, format, args);

It needs to be surrounded in va_start, va_end block.

The second subtle error is that vsnprintf returns the size _excluding
the null byte_. This will cause the vsprintf call to overflow the buffer
by 1 byte.

This might work (untested):
 va_start(args, format);
 size_t size = vsnprintf(nullptr, 0, format, args) + 1;
 va_end(args);

Some alternative implementations. The varardic template solution (the
third one) is similar to this code (and the one I like the most):
https://stackoverflow.com/questions/2342162/stdstring-formatting-like-sprintf

James



signature.asc
Description: OpenPGP digital signature
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Re: giada - new release

[Jaromír Mikeš]

2018-01-16 23:26 GMT+01:00 Jaromír Mikeš :

>
>
> 2018-01-16 22:47 GMT+01:00 James Cowgill :
>
>> Hi,
>>
>> On 16/01/18 18:53, Jaromír Mikeš wrote:
>> > Hi,
>> >
>> > in giada package 02-rtmidi-pkgconfig.patch is not applying anymore.
>> > I am not sure if this patch can be dropped or it should be updated?
>> > Can someone have a look please? James maybe you as as author of this
>> patch.
>>
>> Here's a refreshed patch.
>>
>> It looks like upstream has changed the headers to 
>> which is a hack and against the intentions of upstream rtmidi, but it
>> will work without my patch (for now). I think using pkg-config is still
>> more correct though.
>>
>
> ​Thank you James!
>

​Hi,

giada unfortunately fail to build on some archs ... I already informed
upstream but not answer yet.
Can someone look if fixing this is rather trivial or complicated.

https://buildd.debian.org/status/package.php?p=giada
​
​best regards

mira​
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Re: giada - new release

[Jaromír Mikeš]

2018-01-16 22:47 GMT+01:00 James Cowgill :

> Hi,
>
> On 16/01/18 18:53, Jaromír Mikeš wrote:
> > Hi,
> >
> > in giada package 02-rtmidi-pkgconfig.patch is not applying anymore.
> > I am not sure if this patch can be dropped or it should be updated?
> > Can someone have a look please? James maybe you as as author of this
> patch.
>
> Here's a refreshed patch.
>
> It looks like upstream has changed the headers to 
> which is a hack and against the intentions of upstream rtmidi, but it
> will work without my patch (for now). I think using pkg-config is still
> more correct though.
>

​Thank you James!

mira​
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Re: giada - new release

[James Cowgill]

Hi,

On 16/01/18 18:53, Jaromír Mikeš wrote:
> Hi,
> 
> in giada package 02-rtmidi-pkgconfig.patch is not applying anymore.
> I am not sure if this patch can be dropped or it should be updated?
> Can someone have a look please? James maybe you as as author of this patch.

Here's a refreshed patch.

It looks like upstream has changed the headers to 
which is a hack and against the intentions of upstream rtmidi, but it
will work without my patch (for now). I think using pkg-config is still
more correct though.

James
From: James Cowgill 
Date: Wed, 25 Oct 2017 14:25:50 +0200
Subject: build with new rtmidi lib.

--- a/Makefile.am
+++ b/Makefile.am
@@ -70,13 +70,13 @@ endif
 if LINUX
 
 # Add preprocessor flags to enable ALSA, Pulse and JACK in RtAudio.
-cppFlags += -D__LINUX_ALSA__ -D__LINUX_PULSE__ -D__UNIX_JACK__
+cppFlags += $(RTMIDI_CFLAGS)
 
 # Don't stop on JUCE's unused functions.
 cxxFlags += -Wno-error=unused-function
 
 ldAdd += -lsndfile -lfltk -lXext -lX11 -lXft -lXpm -lm -ljack -lasound \
-  -lpthread -ldl -lpulse-simple -lpulse -lsamplerate -lrtmidi -ljansson \
+  -lpthread -ldl -lpulse-simple -lpulse -lsamplerate $(RTMIDI_LIBS) -ljansson \
   -lfreetype
 
 endif
--- a/configure.ac
+++ b/configure.ac
@@ -118,23 +118,7 @@ AC_CHECK_HEADER(
 )
 AC_LANG_POP
 
-if test "x$os" = "xosx"; then
-	AC_LANG_PUSH([C++])
-	AC_CHECK_HEADER(
-		[RtMidi.h],
-		[],
-		[AC_MSG_ERROR([library 'rtMidi' not found!])]
-	)
-	AC_LANG_POP
-else
-	AC_LANG_PUSH([C++])
-	AC_CHECK_HEADER(
-		[rtmidi/RtMidi.h],
-		[],
-		[AC_MSG_ERROR([library 'rtMidi' not found!])]
-	)
-	AC_LANG_POP
-fi
+PKG_CHECK_MODULES([RTMIDI], [rtmidi])
 
 
 AC_LANG_PUSH([C++])


signature.asc
Description: OpenPGP digital signature
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

giada - new release

[Jaromír Mikeš]

Hi,

in giada package 02-rtmidi-pkgconfig.patch is not applying anymore.
I am not sure if this patch can be dropped or it should be updated?
Can someone have a look please? James maybe you as as author of this patch.

best regards

mira
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers