| Message ID | alpine.DEB.2.21.1911202147000.2011@tglase.lan.tarent.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix segfault on x32, 64-bit time_t-related format strings | expand |
On Wed, 20 Nov 2019 21:51:14 +0100, Thorsten Glaser wrote: > > Hi everyone, > > here are two patches I’ve been rebasing with each new upload > of alsa-lib to Debian to make things work for my x32 desktop. > > The small one fixes it misdetecting x32 as amd64, which leads > to an immediate segfault. > > The other one fixes issues related to printing time_t values > on platforms where time_t is 64 bit wide and long 32 bit wide > (various BSDs and all new 32-bit and *64ilp32 Linux platforms). > > The Debian packager of alsa-lib hasn’t found it necessary to > forward them upstream, so I’m doing it with this. I hope they > can be included in the next releases. The patches were last > updated for 1.1.8 although I need to update them to 1.1.9 which > was recently introduced to Debian… usually they apply still. Could you split these changes to separate patches? They are for different purposes. Also, using time_t would be better if possible. Unfortunately a cast is needed for printf usage, but other than that, time_t would leave us the right size. thanks, Takashi > > Thanks in advance, > //mirabilos > -- > tarent solutions GmbH > Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/ > Tel: +49 228 54881-393 • Fax: +49 228 54881-235 > HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941 > Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg > > ********** > > Mit der tarent Academy bieten wir auch Trainings und Schulungen in den > Bereichen Softwareentwicklung, Agiles Arbeiten und Zukunftstechnologien an. > > Besuchen Sie uns auf www.tarent.de/academy. Wir freuen uns auf Ihren Kontakt. > > ********** > # DP: fix long vs. long long confusion when there is a 64-bit time_t > # DP: on a 32-bit long system, such as all newer 32-bit architectures > > --- a/src/pcm/pcm.c > +++ b/src/pcm/pcm.c > @@ -2257,11 +2257,11 @@ int snd_pcm_status_dump(snd_pcm_status_t > { > assert(status); > snd_output_printf(out, " state : %s\n", snd_pcm_state_name((snd_pcm_state_t) status->state)); > - snd_output_printf(out, " trigger_time: %ld.%06ld\n", > - status->trigger_tstamp.tv_sec, > - status->trigger_tstamp.tv_nsec / 1000); > - snd_output_printf(out, " tstamp : %ld.%06ld\n", > - status->tstamp.tv_sec, status->tstamp.tv_nsec / 1000); > + snd_output_printf(out, " trigger_time: %lld.%06ld\n", > + (long long)status->trigger_tstamp.tv_sec, > + (long)status->trigger_tstamp.tv_nsec / 1000L); > + snd_output_printf(out, " tstamp : %lld.%06ld\n", > + (long long)status->tstamp.tv_sec, (long)status->tstamp.tv_nsec / 1000L); > snd_output_printf(out, " delay : %ld\n", (long)status->delay); > snd_output_printf(out, " avail : %ld\n", (long)status->avail); > snd_output_printf(out, " avail_max : %ld\n", (long)status->avail_max); > --- a/test/latency.c > +++ b/test/latency.c > @@ -325,12 +325,12 @@ void setscheduler(void) > printf("!!!Scheduler set to Round Robin with priority %i FAILED!!!\n", sched_param.sched_priority); > } > > -long timediff(snd_timestamp_t t1, snd_timestamp_t t2) > +long long timediff(snd_timestamp_t t1, snd_timestamp_t t2) > { > - signed long l; > + signed long long l; > > t1.tv_sec -= t2.tv_sec; > - l = (signed long) t1.tv_usec - (signed long) t2.tv_usec; > + l = (signed long long) t1.tv_usec - (signed long long) t2.tv_usec; > if (l < 0) { > t1.tv_sec--; > l = 1000000 + l; > @@ -682,10 +682,10 @@ int main(int argc, char *argv[]) > snd_pcm_nonblock(phandle, !block ? 1 : 0); > if (ok) { > #if 1 > - printf("Playback time = %li.%i, Record time = %li.%i, diff = %li\n", > - p_tstamp.tv_sec, > + printf("Playback time = %lli.%i, Record time = %lli.%i, diff = %lli\n", > + (long long)p_tstamp.tv_sec, > (int)p_tstamp.tv_usec, > - c_tstamp.tv_sec, > + (long long)c_tstamp.tv_sec, > (int)c_tstamp.tv_usec, > timediff(p_tstamp, c_tstamp)); > #endif > --- a/test/queue_timer.c > +++ b/test/queue_timer.c > @@ -99,11 +99,11 @@ main(int argc ATTRIBUTE_UNUSED, char **a > normalize(&diffdiff); > prevdiff = diff; > > - fprintf(stderr, " real time: %12ld sec %8ld usec\nqueue time: %12ld sec %8ld usec\n diff: %12ld sec %8ld usec\n diffdiff: %12ld sec %8ld usec\n", > - tv.tv_sec, tv.tv_usec, > - (long)rtime->tv_sec, (long)rtime->tv_nsec / 1000, > - diff.tv_sec, diff.tv_usec, > - (long)diffdiff.tv_sec, (long)diffdiff.tv_usec); > + fprintf(stderr, " real time: %12lld sec %8ld usec\nqueue time: %12lld sec %8ld usec\n diff: %12lld sec %8ld usec\n diffdiff: %12lld sec %8ld usec\n", > + (long long)tv.tv_sec, (long)tv.tv_usec, > + (long long)rtime->tv_sec, (long)rtime->tv_nsec / 1000, > + (long long)diff.tv_sec, (long)diff.tv_usec, > + (long long)diffdiff.tv_sec, (long)diffdiff.tv_usec); > > if (diffdiff.tv_usec > 5000 || > diffdiff.tv_usec < -5000) { > # DP: fix segmentation fault coming from this using amd64 assembly code > # DP: on x32 systems > > --- a/src/pcm/pcm_dmix.c > +++ b/src/pcm/pcm_dmix.c > @@ -145,7 +145,7 @@ static void dmix_server_free(snd_pcm_dir > #include "pcm_dmix_generic.c" > #if defined(__i386__) > #include "pcm_dmix_i386.c" > -#elif defined(__x86_64__) > +#elif defined(__x86_64__) && !defined(__ILP32__) > #include "pcm_dmix_x86_64.c" > #else > #ifndef DOC_HIDDEN > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Thu, 21 Nov 2019, Takashi Iwai wrote: > Could you split these changes to separate patches? They are for > different purposes. I’ve sent them as two separate patch files attachments. > Also, using time_t would be better if possible. Unfortunately a cast > is needed for printf usage, but other than that, time_t would leave us > the right size. In timediff() you mean? Hrm, indeed. I tried to be minimal-invasive at first. bye, //mirabilos
On Thu, 21 Nov 2019 15:37:46 +0100, Thorsten Glaser wrote: > > On Thu, 21 Nov 2019, Takashi Iwai wrote: > > > Could you split these changes to separate patches? They are for > > different purposes. > > I’ve sent them as two separate patch files attachments. Then please make them cleanly applicable via git-am, with a proper subject, a proper changelog and your sign-off, etc. At best send one patch per mail (and with a cover letter for multiple patches). > > Also, using time_t would be better if possible. Unfortunately a cast > > is needed for printf usage, but other than that, time_t would leave us > > the right size. > > In timediff() you mean? Hrm, indeed. I tried to be minimal-invasive > at first. I meant using time_t as much as possible instead of long long. For printf(), there is no other option, but other calculations can be replaced with time_t. thanks, Takashi
On Fri, 22 Nov 2019, Takashi Iwai wrote: > > I’ve sent them as two separate patch files attachments. > > Then please make them cleanly applicable via git-am, with a proper > subject, a proper changelog and your sign-off, etc. I’ll resend them like that now. Sorry it took a bit, I was busy. > At best send one patch per mail (and with a cover letter for multiple > patches). This is a bit tricky, as I can’t easily inject git format-patch output into this mail setup. I will try sending them from home. > > > Also, using time_t would be better if possible. Unfortunately a cast > > > is needed for printf usage, but other than that, time_t would leave us > > > the right size. > > > > In timediff() you mean? Hrm, indeed. I tried to be minimal-invasive > > at first. > > I meant using time_t as much as possible instead of long long. I’ve done this now. bye, //mirabilos
# DP: fix segmentation fault coming from this using amd64 assembly code # DP: on x32 systems --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -145,7 +145,7 @@ static void dmix_server_free(snd_pcm_dir #include "pcm_dmix_generic.c" #if defined(__i386__) #include "pcm_dmix_i386.c" -#elif defined(__x86_64__) +#elif defined(__x86_64__) && !defined(__ILP32__) #include "pcm_dmix_x86_64.c" #else #ifndef DOC_HIDDEN