[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#8545: closed (issues with recent doprnt-related changes)

From: GNU bug Tracking System
Subject: bug#8545: closed (issues with recent doprnt-related changes)
Date: Mon, 14 Sep 2020 18:42:02 +0000

Your message dated Mon, 14 Sep 2020 21:41:56 +0300
with message-id <83bli89p57.fsf@gnu.org>
and subject line Re: bug#8545: Merged fixes for 8600, 8601, 8602, and 
(partially) for 8545
has caused the debbugs.gnu.org bug report #8545,
regarding issues with recent doprnt-related changes
to be marked as done.

(If you believe you have received this mail in error, please contact

8545: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8545
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
--- Begin Message --- Subject: issues with recent doprnt-related changes Date: Sun, 24 Apr 2011 22:46:33 -0700 User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20110223 Thunderbird/3.1.8
This is a followup to Bug#8435.  Eli invited me to review the recent
doprnt-related changes, so here's a quick review:

* doprnt returns size_t.  But Stefan wrote that he prefers sizes to be
  signed values, and doprnt always returns a value that can fit in
  EMACS_INT.  So shouldn't doprnt return EMACS_INT, as it did before?

* doprnt supports only a small subset of the standard printf formats,
  but this subset is not documented.  It's unclear what the subset is.
  Or it's a superset of the subset, with %S and %l?  Anyway, this
  should be documented clearly in the lead comment.

* I suggest that every feature in doprnt be a feature that is actually
  needed and used; this will simplify maintainance.

* Format strings never include embedded null bytes, so there's
  no need for doprnt to support that.

* If the format string is too long, the alloca inside doprnt will
  crash Emacs on some hosts.  I suggest removing the alloca,
  instituting a fixed size limit on format specifiers, and documenting
  that limit.  Since user code cannot ever supply one of these
  formats, that should be good enough.

* The width features of doprnt (e.g., %25s) are never used.  That part
  of the code is still buggy; please see some comments below.  I
  suggest removing it entirely; this will simplify things.  But if not:

  - doprnt mishandles format specifications such as %0.0.0d.
    It passes them off to printf, and this results in undefined
    behavior, near as I can tell.

  - doprnt uses atoi (&fmtcpy[1]), but surely this isn't right if
    there are flags such as '-'.

  - Quite possibly there are other problems in this area, but I
    didn't want to spend further time reviewing a never-used feature.

* In this code, in verror:

      used = doprnt (buffer, size, m, m + mlen, ap);

      /* Note: the -1 below is because `doprnt' returns the number of bytes     
         excluding the terminating null byte, and it always terminates with a   
         null byte, even when producing a truncated message.  */
      if (used < size - 1)

  I don't see the reason for the "- 1".  If you replace this with:

      used = doprnt (buffer, size, m, m + mlen, ap);

      if (used < size)

  the code should still work, because, when used < size, the buffer
  should be properly null-terminated.  If it isn't then there's something
  wrong with doprnt, no?

* In this code, in verror:

      else if (size < size_max - 1)
        size = size_max - 1;

  there's no need for the "- 1"s.  Just use this:

      else if (size < size_max)
        size = size_max;

* This code in verror:

      if (buffer == buf)
        buffer = (char *) xmalloc (size);
        buffer = (char *) xrealloc (buffer, size);

  uses xrealloc, which is unnecessarily expensive, as it may copy the
  buffer's contents even though they are entirely garbage here.  Use
  this instead, to avoid the useless copy:

      if (buffer != buf)
        xfree (buffer);
      buffer = (char *) xmalloc (size);

--- End Message ---
--- Begin Message --- Subject: Re: bug#8545: Merged fixes for 8600, 8601, 8602, and (partially) for 8545 Date: Mon, 14 Sep 2020 21:41:56 +0300
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 14 Sep 2020 14:37:44 +0200
> Cc: 8545@debbugs.gnu.org
> This was a quite long thread, and had a bunch of related bugs discussed.
> Skimming this thread, I'm not sure exactly what the remaining problem in
> this bug report is -- the discussion kinda petered out...
> This is nine years old -- is there anything more to be done here?

I don't think so, so I'm closing this bug.

--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]