bug#9031: Unused vars etc. in chartab.c, composite.c, gtkutil.c

bug#9031: Unused vars etc. in chartab.c, composite.c, gtkutil.c

[Paul Eggert]

This patch fixes some minor problems on the Emacs trunk.

Normally I'd just install this, but we're in a feature freeze
now and this patch doesn't fix any actual bugs, so I'm filing
a bug report instead, to record the issues.

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: eggert@cs.ucla.edu-20110709062840-jdrk1zrclfflltae
# target_branch: bzr+ssh://eggert@bzr.savannah.gnu.org/emacs/trunk
# testament_sha1: 935c16305b271d2f38af33043c16de6e23c306ca
# timestamp: 2011-07-08 23:28:47 -0700
# base_revision_id: sdl.web@gmail.com-20110709031157-d7vix3jz1gpb29bw
# 
# Begin patch
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-07-08 20:39:30 +0000
+++ src/ChangeLog	2011-07-09 06:28:40 +0000
@@ -1,5 +1,12 @@
 2011-07-08  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Fix minor problems found by static checking.
+	* chartab.c (char_table_set_range, map_sub_char_table)
+	(uniprop_table_uncompress): Remove unused locals.
+	(uniprop_table): Now static.
+	* composite.c (_work_char): Remove unused static var.
+	* gtkutil.c (qttip_cb): Remove stray no-effect statement.
+
 	Use pthread_sigmask, not sigprocmask (Bug#9010).
 	sigprocmask is portable only for single-threaded applications, and
 	Emacs can be multi-threaded when it uses GTK.

=== modified file 'src/chartab.c'
--- src/chartab.c	2011-07-07 04:16:52 +0000
+++ src/chartab.c	2011-07-09 06:28:40 +0000
@@ -485,7 +485,6 @@
 char_table_set_range (Lisp_Object table, int from, int to, Lisp_Object val)
 {
   struct Lisp_Char_Table *tbl = XCHAR_TABLE (table);
-  Lisp_Object *contents = tbl->contents;
 
   if (from == to)
     char_table_set (table, from, val);
@@ -759,8 +758,6 @@
 		    Lisp_Object function, Lisp_Object table, Lisp_Object arg, Lisp_Object val,
 		    Lisp_Object range, Lisp_Object top)
 {
-  /* Pointer to the elements of TABLE. */
-  Lisp_Object *contents;
   /* Depth of TABLE.  */
   int depth;
   /* Minimum and maxinum characters covered by TABLE. */
@@ -777,14 +774,12 @@
       struct Lisp_Sub_Char_Table *tbl = XSUB_CHAR_TABLE (table);
 
       depth = XINT (tbl->depth);
-      contents = tbl->contents;
       min_char = XINT (tbl->min_char);
       max_char = min_char + chartab_chars[depth - 1] - 1;
     }
   else
     {
       depth = 0;
-      contents = XCHAR_TABLE (table)->contents;
       min_char = 0;
       max_char = MAX_CHAR;
     }
@@ -1143,7 +1138,6 @@
   Lisp_Object sub = make_sub_char_table (3, min_char, Qnil);
   struct Lisp_Sub_Char_Table *subtbl = XSUB_CHAR_TABLE (sub);
   const unsigned char *p, *pend;
-  int i;
 
   XSUB_CHAR_TABLE (table)->contents[idx] = sub;
   p = SDATA (val), pend = p + SBYTES (val);
@@ -1316,7 +1310,7 @@
    function may load a Lisp file and thus may cause
    garbage-collection.  */
 
-Lisp_Object
+static Lisp_Object
 uniprop_table (Lisp_Object prop)
 {
   Lisp_Object val, table, result;

=== modified file 'src/composite.c'
--- src/composite.c	2011-07-07 16:18:25 +0000
+++ src/composite.c	2011-07-09 06:28:40 +0000
@@ -967,7 +967,6 @@
 }
 
 static Lisp_Object _work_val;
-static int _work_char;
 
 /* 1 iff the character C is composable.  Characters of general
    category Z? or C? are not composable except for ZWNJ and ZWJ. */

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2011-07-08 17:57:55 +0000
+++ src/gtkutil.c	2011-07-09 06:28:40 +0000
@@ -647,7 +647,6 @@
       /* Change stupid Gtk+ default line wrapping.  */
       p = gtk_widget_get_parent (x->ttip_lbl);
       list = gtk_container_get_children (GTK_CONTAINER (p));
-      iter;
       for (iter = list; iter; iter = g_list_next (iter))
         {
           GtkWidget *w = GTK_WIDGET (iter->data);

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWeoOTFQABHpfgAAwUHf/91sH
0AC////wYAf99dikKUFEaHR0AAUCGpEyaaM9U0ZpMmho09TTJhMjQwhhJIIYJoRiVPU8jKeUGg0a
aMmmIOYAAAAAAAAAAGJFT/VTwBT00j1MgDIaPUyD0RoA5gAAAAAAAAAAKkkBExGmmjIGpphNTE0m
1GTTR5RAyujahbZcL/mRiMY+gvwE7KK46KZmEAYi+mDO5Z0mOeWxa9GjHPRa1sbsHBdwvvaMX0Kp
1FGIIpEJJAJY+pN0Iuap4SwYimt4OqVzO3BXGSLrF7Peqzcptkyqe8t6gP3O8eMcB7/qfIfAwXif
U7GT+7FKTWuYvw0MB/yxrdPDt7eHbWGL+qwOxoLY8b/0GHlYj1k9fKqwZ0/27nBjjVVWD/w2HOpK
6r7y0qQ8lPLrs6ZM13VPZ6PB1h/biF6jrrTrk+EIQyLbG56JzmHDizlb5IF6QJVQLCdpomHPVWjo
vke3K0N9O+p11yYROku4xNKmVRJWONLmmUZ5rExvkKwwqJK/tUWdliRwCsT8xks0DNijLlj6BkED
luKbCfOd4JaRCXmfl0PdFCWL+wbeJnT2hVCVgTTBXoUTcUKln5Q4xlDnsXaOpUxBUIOYco/SHAED
Uf3qJobwXU8CufO+HEam2mXn+vFg6nlV1gXXkuQIhwGc7PBw8Xf1u90fmWOJOvjqrLo4ksqJOLA6
SdmWBiQNO6ccUomBm+MqIwglzvIq8fORgCYOR+eR0XJ2jUmgvMbwqaUGMDTKUrxydNLLAUSVVg/o
iqUTFZ0bF80OphWlfu1Ya2DBr1WYVa/iutYiZhN2RDOpA7/0U+gVrsGMYyBNm9BxeQKsiMBiDgT+
jQkbmeM1sFtSskuBaHShHXpc6eSVvRKZHOUdUpjaXsRwN/Cbzk2NdtyzF8+s230VHuIa1eWgbQx1
b7gUIl0un3gvCdLlLyLkzuxMktXpBiYcMTN2yv3TalOpCJQ4GglaDrcl7Sxy5zZax1zVkkl+ON2y
7rys9jg05KdDG2PVsaFMFeWNtTLcSAoX7EXFAXrfgulLOCiZ7RzcUYvSCcEXMNyVyBYnrJGW5lvN
PbMbUvAzLWDLFiY8s/FiLtoPQsBI7MClIk8qYD5ZA84HlzHAiQ672aq1bVbmO323NMprZSa5uWWb
drZy9fSR9hlqcxbbS3Upoc1ka3FTNdmuSywpkylsE2tEM8FOp7psdk01VU4Wqqe5Vqjl+5XJlCdp
zuOEzJEIBIbZGoHl2NBmH6wYHA6BQHNgVQChT6/G5cMn0UpS5/l6QmjInc9G+uFs27b18a9+iLlD
nysTQ2tsPOY/r7N5m/bJupUzbbPyo0pKi+ZKVD8Mn05miPXoaZ5vd8ny7G/3zns9+K/k683Nhztc
n4sPs8V8zJ6v1T5yHKLTiCQMmATzpiA1h5Q6KTGRqP02CAOiUEorAQXYCA3OWg/RxXX1Zta+/Obj
ZCcScTnaGnmrY/qE0TXleujW7vd29u8wb+lVmiTjpaRql7sZo++6/jyeLwas/ixa3T3bfNN8/Qmh
lCfCifvm+p9/VLouDyX5ogoA2p/VfZ9kvAaPGu8pdTie5kcckD2h+wOXZU6lnR1FQo3Ptobp9Ljb
pfHgb1njCayXW8sSdva1abOh1fH4vM9ntYstFFv6erTtnjyst0f0uPhzqk8H5Txff6vJR7JQ3+eb
8fDVInyfNm0m/FcjNF70efW83y+F7ft67+DjE+VPD5Wk1LMubH0YLmY2ErY6kzW7icj1P1MicLTo
VKfScL2uS1nOqoUd0ifw2Qn2nPOrU9vau588Lcibf+3RLVHrvGt5TdUK6X1+b7o9bMXmlRwncSxv
1w10er7+LpYOpGby+3SfNH4xjmfhfLj0/uauolnpT7zv1U7ieNLX90LGZ0xx/h7jvnIdG1FLDRCY
HQUKSxM7+DwJpeC9g0JPePbYYSe8ybozL3Y5zB2tLBMWKXSXMbLX+FwyLmsxha95XS6UpvYRivfT
kTSkuoYFk6e5mTnblcrLdcTJH7LpMm+Htrc0bb0qOm5vT8s7O7U3sjfd2LFDhMk/nF15a8IupmvM
30NRPPo9mzUo7Si0OTyMDYOwLAS1LHiaCnQn5AnpYlOYvFgkKaUxxpNzW73fCfExvZOed/AtrVOV
KqxxRrixom2pVL/maU3OqjQpUKeehd4XcpOnf8Fngc38+OLGEwMHsLo1DiThE6YwZtzaMVzfUNDs
jcZN0foaoc6fay2rRYbHSw6hwkij3LLld0J0OTe3Pia9h06ec2xeu+RM0wTgj3m3uwejbK0UTnbp
NxNm+Ppz5ybCNjcoOD3JXp/a/nGqJHsBf+LuSKcKEh1ByYqA

bug#9031: Installed rest of patch

[Paul Eggert]

I see that I was too cautious, in that some of my patch
was derived independently and installed into the trunk
as part of bzr 105059.  I just now installed the rest of
the patch and am marking this bug report as done.

bug#9031: Installed rest of patch

[Juanma Barranquero]

On Sun, Jul 10, 2011 at 07:21, Paul Eggert <eggert@cs.ucla.edu> wrote:

> I see that I was too cautious

Removing dead code and unused variables is not a new feature, and we
haven't even started pretest yet as there have been no pretest
releases.

    Juanma

bug#9031: Installed rest of patch

[Stefan Monnier]

>> I see that I was too cautious
> Removing dead code and unused variables is not a new feature, and we
> haven't even started pretest yet as there have been no pretest
> releases.

At the same time, we're in feature freeze, which means we want to limit
code changes to bug fixes because we'd rather not add more bugs at
this stage.  So it's better to keep code cleanups for later (you could
try to install them in the `pending' branch, but such code cleanups are
likely to generate merge conflicts).


        Stefan

bug#9031: Installed rest of patch

[Paul Eggert]

On 07/11/11 05:49, Stefan Monnier wrote:
> it's better to keep code cleanups for later

This particular fix is both a code cleanup and a bug fix.
The latter, because often compilers complain about unused
local variables, and we'd rather have Emacs builds that
don't have useless chatter and generate bug reports that
waste everybody's time later.

Compiler warnings are a judgment call.  Pacifying
every compiler would be a never-ending task, and we shouldn't
try to do that.  But this particular case is a trivial and
obviously-safe change, and it fixes GCC warnings about
code that was introduced after the feature freeze.
The (admittedly minor) benefit of such a change surely
exceeds its cost.

I'd prefer it if, when one builds Emacs, one gets no more
compile-time diagnostics than one did when the feature
freeze was introduced.  This shouldn't be an absolute goal
and it shouldn't stand in the way of more-important issues,
but surely it's not an unreasonable aspiration.

> you could
> try to install them in the `pending' branch, but such code cleanups are
> likely to generate merge conflicts

Does this advice mean that code cleanups should not be installed
anywhere now, not even in the 'pending' branch?  I ask because I
have some integer-overflow-and-signedness-related cleanups and
bug-fixes in my private copy that should really get incorporated
at some point.

(Sorry, I don't know the current practice with regards to the
'pending' branch; maybe 'pending' should be documented somewhere
under admin/?)

bug#9031: Installed rest of patch

[Stefan Monnier]

>> it's better to keep code cleanups for later
> This particular fix is both a code cleanup and a bug fix.

Yes, it was OK.  I just felt that Juanma's answer was a bit too much
on the "we're not yet in pretest" side, so I wanted to compensate by
reminding people that we should think of fixing bugs now, not cleaning
things up.

>> you could try to install them in the `pending' branch, but such code
>> cleanups are likely to generate merge conflicts
> Does this advice mean that code cleanups should not be installed
> anywhere now, not even in the 'pending' branch?

You can install them there, but that branch is really meant to be
a repository of patches that will get merged to trunk in some distant
future.  Code that touches a lot of code is likely to encounter merge
conflicts when we get to that distant future.  And changes which need to
apply to "all places where foo happens" are even more likely to be
out-of-date when we get to that distant future.

> I ask because I have some integer-overflow-and-signedness-related
> cleanups and bug-fixes in my private copy that should really get
> incorporated at some point.

Put them somewhere.  But I suspect that it'll be easier to re-do them
than to merge the current patch several months from now, so it's
probably important to store somewhere the "how did I come up with this
patch" along with the patch.

> (Sorry, I don't know the current practice with regards to the
> 'pending' branch; maybe 'pending' should be documented somewhere
> under admin/?)

It's "documented" in http://bzr.savannah.gnu.org/r/emacs/README.
We don't really want to encourage people to use it too much, since the
focus should instead move to fixing bugs.  But sometimes it's easier to
install the patch somewhere than to try to mark some bug report as "it's
got a patch and the patch is good to go, we just have to wait for the
trunk to open up again before we can install it".


        Stefan

bug#9031: Installed rest of patch

[Paul Eggert]

>> I ask because I have some integer-overflow-and-signedness-related
>> cleanups and bug-fixes in my private copy...
> Put them somewhere.  But I suspect that it'll be easier to re-do them
> than to merge the current patch several months from now, so it's
> probably important to store somewhere the "how did I come up with this
> patch" along with the patch.

Unfortunately the answer to "how did I come up with this patch?" is,
"I read the Emacs source code carefully and found dozens of problems".
It was done all by hand.

As the patch in question should be entirely a bug-fix patch, and should
not add any features or establish any new dependencies on system functions,
I'm hoping that it will be preferable to put it on the trunk now rather than
months from now.  But we can cross that bridge once the patch is ready
and you have a chance to look at it.

It sounds like an Emacs bug report is the next step to take in publishing the
patch, so I'll work on putting that together.  I had hoped to do a complete sweep
of the Emacs source code before publishing the patch, but given the above
it sounds like I should publish what I have now, as there's
little sense finding and fixing more bugs if I'll have to do it all
over again in 2012.

bug#9031: Installed rest of patch

[Juanma Barranquero]

On Tue, Jul 12, 2011 at 06:51, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Yes, it was OK.  I just felt that Juanma's answer was a bit too much
> on the "we're not yet in pretest" side, so I wanted to compensate by
> reminding people that we should think of fixing bugs now, not cleaning
> things up.

Believe it or not, I'm quite conservative with respect to pretests.

But... a formal announcemente about which state are we just now and
when do we <em>expect</em> to release the first pretest tarball would
be welcome.

    Juanma

bug#9031: Installed rest of patch

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

> But... a formal announcemente about which state are we just now and
> when do we <em>expect</em> to release the first pretest tarball would
> be welcome.

That depends on the bidi changes.