Re: Texinfo 6.7 released

2019-09-28 Thread Raymond Toy
Thanks for the fix.  Everything works just fine now!

On Fri, Sep 27, 2019 at 4:54 AM Gavin Smith 
wrote:

> On Fri, Sep 27, 2019 at 7:09 AM Raymond Toy  wrote:
> > I can build texinfo from these files without any problems.
> >
> > However, if I use the git repo I can't.  I checked out the tag
> texinfo-6.7 and ran autogen.sh and get this error:
> >
> > Preparing Texinfo development infrastructure:
> >   ./tp/maintain/regenerate_file_lists.pl
> >   (cd tp && ./maintain/regenerate_docstr.sh Makefile.docstr)
> >   (cd tp/tests && ../maintain/regenerate_cmd_tests.sh Makefile.onetst .
> -base 'formatting sectioning indices nested_formats contents layout'
> -tex_html 'tex_html')
> >   aclocal -I gnulib/m4 && autoconf && autoheader && automake
> > missing file gnulib/lib/windows-mutex.c
> > configure.ac:90: error: expected source file, required through
> AC_LIBSOURCES, not found
> > gnulib/m4/gnulib-comp.m4:156: gl_INIT is expanded from...
> > configure.ac:90: the top level
> > autom4te: /usr/bin/m4 failed with exit status: 1
> > aclocal: error: echo failed with exit status: 1
>
>
> Thank for the report. What I have done is committed the files and
> moved the texinfo-6.7 tag. If you delete the local copy of the tag and
> check it out again, it should work now.
>


-- 
Ray


Re: Texinfo 6.7 released

2019-09-28 Thread Eli Zaretskii
> Date: Sat, 28 Sep 2019 13:53:35 +0300
> From: Eli Zaretskii 
> Cc: bug-texinfo@gnu.org
> 
> I will run the test suite now.

All the tests passed, with some skipped (as expected).

Thanks.



Re: Texinfo 6.7 released

2019-09-28 Thread Eli Zaretskii
> From: Gavin Smith 
> Date: Sat, 28 Sep 2019 10:56:07 +0100
> Cc: Texinfo 
> 
> On Sat, Sep 28, 2019 at 9:04 AM Eli Zaretskii  wrote:
> > For the record, the version of Perl I used to build the pretest is
> > 5.22.1.  But I'm not sure it is relevant to the issue at hand, see my
> > other message.  Maybe it was just sheer luck that Perl didn't crash
> > for me while building the pretest, or maybe the Windows port of Perl
> > was changed in later versions so as not to bump into this problem when
> > extensions allocate storage.
> 
> Another possibility is that you had TEXINFO_XS_PARSER set in the
> environment which caused the Parsetexi module to be loaded.

Does that setting disable loading the other modules, where the problem
happened?

I do have that setting in my ~/.bash_profile file on the system where
I complied 6.7, and where Perl crashed.  Does the build disable that
in some way?

I will see if I have any setting of TEXINFO_XS_PARSER on the system
where I built the pretest.



Re: Texinfo 6.7 released

2019-09-28 Thread Eli Zaretskii
> From: Gavin Smith 
> Date: Sat, 28 Sep 2019 11:20:29 +0100
> Cc: Texinfo 
> 
> > I needed to add dTHX to 'init', or it wouldn't compile.
> 
> Didn't it compile before?

It did, because there was no stderr in that function.  Once I removed
the printf's, the dTHX is no longer needed in that function.

> > --- ./tp/Texinfo/XS/parsetexi/api.c.~1~ 2019-08-25 20:11:45.0 +0300
> > +++ ./tp/Texinfo/XS/parsetexi/api.c 2019-09-28 10:50:13.319625000 +0300
> > @@ -56,7 +56,8 @@ find_locales_dir (char *builddir)
> >
> >dTHX;
> >
> > -  asprintf (, "%s/LocaleData", builddir);
> > +  s = malloc (strlen (builddir) + strlen ("/LocaleData") + 1);
> > +  sprintf (s, "%s/LocaleData", builddir);
> >dir = opendir (s);
> >if (!dir)
> >  {
> >
> >
> > and the problem went away.
> 
> Would you like to commit that change?

Done, with a comment explaining why asprintf cannot be used there.

> I don't know of a good way to avoid using the Perl version of free.

I don't think there is one.  I think we will have to be vigilant and
avoid using Gnulib functions that allocate storage without freeing it,
in files that include Perl headers.

> > If this is the right fix, then I think we
> > cannot use Gnulib's asprintf in Parsetexi, and there are a couple of
> > other places with the same problem, which we need to fix as well,
> > right?
> 
> Hopefully not: only if Perl headers are included in a file should
> there be problems.

Ah, I see that the other grep hits were indeed in files that don't
include Perl headers.  So we are safe, I think.

> I deliberately limited where the Perl headers were included: for
> example, in tree_types.h a field of a struct is declared as a void
> pointer rather than the Perl type it should be. The files where there
> could be problems are api.c and Parsetexi.xs: it shouldn't be too hard
> to avoid using asprintf there (or other functions which use the real
> version of malloc).

Right.

I will run the test suite now.



Re: paragraph justification issue on @url with "texi2dvi --pdf" in texinfo.tex 2019-09-20.22

2019-09-28 Thread Gavin Smith
On Wed, Sep 25, 2019 at 3:03 PM Vincent Lefevre  wrote:
> This fixes some cases, but not others. I've attached a url-break.texi
> file with 4 cases. And I've tested 4 different texinfo.tex solutions
> (the old one, the new one, the new one with this patch, and another
> one with just a \allowbreak). As a summary:

Thanks for preparing the different cases. I've committed a change that
stops the ragged-right output in the cases you sent but keeps it in
others.
\input texinfo@c -*-texinfo-*-
@iftex
@afourpaper
@end iftex

Initial text to test the justification of the line containing the
first part of the URL: blah blah blah blah blah blah blah blah.
@url{https://www.1234567890123456789012345678901234567890123.invalid}.
Text to test the justification of the line containing the last part
of the URL: blah blah blah blah blah blah blah blah.

Initial text to test the justification of the line containing the
first part of the URL: blah blah blah blah blah blah blah blah.
@url{https://www.1234567890123456789012345678901234567890.invalid}.
Text to test the justification of the line containing the last part
of the URL: blah blah blah blah blah blah blah blah.

Initial text to test the justification of the line containing the
first part of the URL: blah blah blah blah blah blah blah blah.
@url{https://www.1234567890123456789012345678901234567.invalid}.
Text to test the justification of the line containing the last part
of the URL: blah blah blah blah blah blah blah blah.

Initial text to test the justification of the line containing the
first part of the URL: blah blah blah blah blah blah blah blah.
@url{https://www.1234567890123456789012345678901234.invalid}.
Text to test the justification of the line containing the last part
of the URL: blah blah blah blah blah blah blah blah.

see the 
@url{https://www.gnu.org/software/libc/manual/html_node/Message-catalogs-with-gettext.html} 

foo foo foo foo foo foo foof see the @url{https://www.gnu.org/software/libc/manual/html_node/}

In case of problem, please read the @file{INSTALL} file carefully
before reporting a bug, in particular section ``In case of problem''.
Some problems are due to bad configuration on the user side (not
specific to MPFR)@. Problems are also mentioned in the FAQ
@url{https://www.mpfr.org/faq.html}.

@bye


url-break.pdf
Description: Adobe PDF document


Re: Texinfo 6.7 released

2019-09-28 Thread Gavin Smith
> Maybe that's what we should do here as well.  'free' is a macro
> expanded into something implemented inside Perl, right?  And asprintf
> comes from Gnulib on MS-Windows, right?  So we have that problem I
> mentioned with allocation and free that come from different libraries.

Yes, that makes sense.

> I needed to add dTHX to 'init', or it wouldn't compile.

Didn't it compile before?

> So it looks like it indeed crashes when calling 'free' on storage
> allocated by asprintf.  With that in mind, I modified api.c as below:
>
> --- ./tp/Texinfo/XS/parsetexi/api.c.~1~ 2019-08-25 20:11:45.0 +0300
> +++ ./tp/Texinfo/XS/parsetexi/api.c 2019-09-28 10:50:13.319625000 +0300
> @@ -56,7 +56,8 @@ find_locales_dir (char *builddir)
>
>dTHX;
>
> -  asprintf (, "%s/LocaleData", builddir);
> +  s = malloc (strlen (builddir) + strlen ("/LocaleData") + 1);
> +  sprintf (s, "%s/LocaleData", builddir);
>dir = opendir (s);
>if (!dir)
>  {
>
>
> and the problem went away.

Would you like to commit that change?

I don't know of a good way to avoid using the Perl version of free.
You could have "#undef free" followed by "#define free PerlMem_free",
but this would break if perl changes the way that it overrides "free".

> If this is the right fix, then I think we
> cannot use Gnulib's asprintf in Parsetexi, and there are a couple of
> other places with the same problem, which we need to fix as well,
> right?

Hopefully not: only if Perl headers are included in a file should
there be problems. I believe you successfully built and used the
Parsetexi module before on MS-Windows when asprintf was used in other
files.

I deliberately limited where the Perl headers were included: for
example, in tree_types.h a field of a struct is declared as a void
pointer rather than the Perl type it should be. The files where there
could be problems are api.c and Parsetexi.xs: it shouldn't be too hard
to avoid using asprintf there (or other functions which use the real
version of malloc).



Re: Texinfo 6.7 released

2019-09-28 Thread Gavin Smith
On Sat, Sep 28, 2019 at 9:04 AM Eli Zaretskii  wrote:
> For the record, the version of Perl I used to build the pretest is
> 5.22.1.  But I'm not sure it is relevant to the issue at hand, see my
> other message.  Maybe it was just sheer luck that Perl didn't crash
> for me while building the pretest, or maybe the Windows port of Perl
> was changed in later versions so as not to bump into this problem when
> extensions allocate storage.

Another possibility is that you had TEXINFO_XS_PARSER set in the
environment which caused the Parsetexi module to be loaded.



Re: Texinfo 6.7 released

2019-09-28 Thread Eli Zaretskii
> From: Gavin Smith 
> Date: Fri, 27 Sep 2019 20:25:38 +0100
> Cc: Texinfo 
> 
> > I cannot check right now, but I think the version of Perl on the
> > system where I built the pretest was indeed above 5.21.5.
> >
> > What is the conditional code above about?  Why is it needed?
> 
> I don't know: that code is automatically generated by xsubpp from 
> Parsetexi.xs.

For the record, the version of Perl I used to build the pretest is
5.22.1.  But I'm not sure it is relevant to the issue at hand, see my
other message.  Maybe it was just sheer luck that Perl didn't crash
for me while building the pretest, or maybe the Windows port of Perl
was changed in later versions so as not to bump into this problem when
extensions allocate storage.



Re: Texinfo 6.7 released

2019-09-28 Thread Eli Zaretskii
> From: Gavin Smith 
> Date: Fri, 27 Sep 2019 20:23:33 +0100
> Cc: Texinfo 
> 
> This problem occurred before and at that time, we solved it by
> avoiding using "free" altogether (commit d9fd777).

Maybe that's what we should do here as well.  'free' is a macro
expanded into something implemented inside Perl, right?  And asprintf
comes from Gnulib on MS-Windows, right?  So we have that problem I
mentioned with allocation and free that come from different libraries.

> > The output is below.  Does it help?
> 
> It shows it is likely the "init" function in parsetexi/api.c where the
> problem occurs. Maybe you could try a change like the one attached and
> see exactly where the problem is occurring.

I needed to add dTHX to 'init', or it wouldn't compile.  After that,
the output is:

  rm -rf $backupdir; exit $rc
  A 1
  A 2
  A 3
  B 1
  B 2
  B 3
  B 7
  B 8
  Free to wrong pool 3f5d48 not 40f018e at ..\tp/Texinfo/XSLoader.pm line 224, 
<$fh> line 8.

So it looks like it indeed crashes when calling 'free' on storage
allocated by asprintf.  With that in mind, I modified api.c as below:

--- ./tp/Texinfo/XS/parsetexi/api.c.~1~ 2019-08-25 20:11:45.0 +0300
+++ ./tp/Texinfo/XS/parsetexi/api.c 2019-09-28 10:50:13.319625000 +0300
@@ -56,7 +56,8 @@ find_locales_dir (char *builddir)
 
   dTHX;
 
-  asprintf (, "%s/LocaleData", builddir);
+  s = malloc (strlen (builddir) + strlen ("/LocaleData") + 1);
+  sprintf (s, "%s/LocaleData", builddir);
   dir = opendir (s);
   if (!dir)
 {


and the problem went away.  If this is the right fix, then I think we
cannot use Gnulib's asprintf in Parsetexi, and there are a couple of
other places with the same problem, which we need to fix as well,
right?

Thanks.