Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [ob] Remove initializations of memset'd structure
@ 2004-02-16 21:15 Daniel Jacobowitz
  2004-02-17  6:54 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Jacobowitz @ 2004-02-16 21:15 UTC (permalink / raw)
  To: gdb-patches

This part of my previous patch is obvious.  Checked in.  [Yes, mst_unknown
is documented to have value 0.]

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-02-16  Daniel Jacobowitz  <drow@mvista.com>

	* objfiles.c (terminate_minimal_symbol_table): Remove unnecessary
	initializations.

--- objfiles.c	14 Feb 2004 15:46:33 -0000	1.49
+++ objfiles.c	15 Feb 2004 23:56:31 -0000
@@ -283,11 +283,6 @@ terminate_minimal_symbol_table (struct o
       = &objfile->msymbols[objfile->minimal_symbol_count];
 
     memset (m, 0, sizeof (*m));
-    DEPRECATED_SYMBOL_NAME (m) = NULL;
-    SYMBOL_VALUE_ADDRESS (m) = 0;
-    MSYMBOL_INFO (m) = NULL;
-    MSYMBOL_SIZE (m) = 0;
-    MSYMBOL_TYPE (m) = mst_unknown;
     SYMBOL_INIT_LANGUAGE_SPECIFIC (m, language_unknown);
   }
 }


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-16 21:15 [ob] Remove initializations of memset'd structure Daniel Jacobowitz
@ 2004-02-17  6:54 ` Eli Zaretskii
  2004-02-17 14:34   ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2004-02-17  6:54 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> Date: Mon, 16 Feb 2004 16:15:16 -0500
> From: Daniel Jacobowitz <drow@false.org>
>  
> -    MSYMBOL_TYPE (m) = mst_unknown;

This part of the patch seems to assume that mst_unknown has the value
zero.  Should we have such assumptions in the code?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17  6:54 ` Eli Zaretskii
@ 2004-02-17 14:34   ` Elena Zannoni
  2004-02-17 19:12     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Elena Zannoni @ 2004-02-17 14:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Jacobowitz, gdb-patches

Eli Zaretskii writes:
 > > Date: Mon, 16 Feb 2004 16:15:16 -0500
 > > From: Daniel Jacobowitz <drow@false.org>
 > >  
 > > -    MSYMBOL_TYPE (m) = mst_unknown;
 > 
 > This part of the patch seems to assume that mst_unknown has the value
 > zero.  Should we have such assumptions in the code?

Yes, in symtab.h mst_unknown is 0. However, that got me puzzled as
well, and I think it warrants a comment. Daniel, please add something
in the function you touched.

elena


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17 14:34   ` Elena Zannoni
@ 2004-02-17 19:12     ` Eli Zaretskii
  2004-02-17 19:37       ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2004-02-17 19:12 UTC (permalink / raw)
  To: Elena Zannoni; +Cc: drow, gdb-patches

> From: Elena Zannoni <ezannoni@redhat.com>
> Date: Tue, 17 Feb 2004 09:30:06 -0500
>  > >  
>  > > -    MSYMBOL_TYPE (m) = mst_unknown;
>  > 
>  > This part of the patch seems to assume that mst_unknown has the value
>  > zero.  Should we have such assumptions in the code?
> 
> Yes, in symtab.h mst_unknown is 0.

I know that, I looked it up in symtab.h.  The question is, should the
code rely on the fact that zeroing out the struct causes the
MSYMBOL_TYPE member to become mst_unknown.  Suppose we change the
enumeration some day, that would break the code.

So I think a comment is not enough, we should leave that line alone.
In general, it is my opinion that code should not assume anything
about the numerical values of enumerated types.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17 19:12     ` Eli Zaretskii
@ 2004-02-17 19:37       ` Elena Zannoni
  2004-02-17 19:47         ` Daniel Jacobowitz
  0 siblings, 1 reply; 8+ messages in thread
From: Elena Zannoni @ 2004-02-17 19:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Elena Zannoni, drow, gdb-patches

Eli Zaretskii writes:
 > > From: Elena Zannoni <ezannoni@redhat.com>
 > > Date: Tue, 17 Feb 2004 09:30:06 -0500
 > >  > >  
 > >  > > -    MSYMBOL_TYPE (m) = mst_unknown;
 > >  > 
 > >  > This part of the patch seems to assume that mst_unknown has the value
 > >  > zero.  Should we have such assumptions in the code?
 > > 
 > > Yes, in symtab.h mst_unknown is 0.
 > 
 > I know that, I looked it up in symtab.h.  The question is, should the
 > code rely on the fact that zeroing out the struct causes the
 > MSYMBOL_TYPE member to become mst_unknown.  Suppose we change the
 > enumeration some day, that would break the code.
 > 

Oh, I see. I misunderstood you. 

 > So I think a comment is not enough, we should leave that line alone.
 > In general, it is my opinion that code should not assume anything
 > about the numerical values of enumerated types.

That's true. It would be safer to leave the line alone. I'll change
it back. Also language_unknown is used similarly.

I've committed this.

2004-02-17  Elena Zannoni  <ezannoni@redhat.com>

	* objfiles.c (terminate_minimal_symbol_table): Add back
	initialization of MSYMBOL_TYPE.


Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.50
diff -u -p -r1.50 objfiles.c
--- objfiles.c  16 Feb 2004 21:14:24 -0000      1.50
+++ objfiles.c  17 Feb 2004 19:36:28 -0000
@@ -283,6 +283,8 @@ terminate_minimal_symbol_table (struct o
       = &objfile->msymbols[objfile->minimal_symbol_count];
  
     memset (m, 0, sizeof (*m));
+    /* Don't rely on these enumeration values being 0's.  */
+    MSYMBOL_TYPE (m) = mst_unknown;
     SYMBOL_INIT_LANGUAGE_SPECIFIC (m, language_unknown);
   }
 }


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17 19:37       ` Elena Zannoni
@ 2004-02-17 19:47         ` Daniel Jacobowitz
  2004-02-17 20:28           ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Jacobowitz @ 2004-02-17 19:47 UTC (permalink / raw)
  To: gdb-patches

On Tue, Feb 17, 2004 at 02:33:27PM -0500, Elena Zannoni wrote:
> Eli Zaretskii writes:
>  > > From: Elena Zannoni <ezannoni@redhat.com>
>  > > Date: Tue, 17 Feb 2004 09:30:06 -0500
>  > >  > >  
>  > >  > > -    MSYMBOL_TYPE (m) = mst_unknown;
>  > >  > 
>  > >  > This part of the patch seems to assume that mst_unknown has the value
>  > >  > zero.  Should we have such assumptions in the code?
>  > > 
>  > > Yes, in symtab.h mst_unknown is 0.
>  > 
>  > I know that, I looked it up in symtab.h.  The question is, should the
>  > code rely on the fact that zeroing out the struct causes the
>  > MSYMBOL_TYPE member to become mst_unknown.  Suppose we change the
>  > enumeration some day, that would break the code.
>  > 
> 
> Oh, I see. I misunderstood you. 
> 
>  > So I think a comment is not enough, we should leave that line alone.
>  > In general, it is my opinion that code should not assume anything
>  > about the numerical values of enumerated types.
> 
> That's true. It would be safer to leave the line alone. I'll change
> it back. Also language_unknown is used similarly.
> 
> I've committed this.

Thanks.  The only downside is that this suggests the type of the
terminating symbol can ever be read without being in error, which is
incorrect; the minimal symbol table's end appears to be marked by
otherwise by SYMBOL_LINKAGE_NAME (msym) == NULL.  Other minimal symbols
may have type mst_unknown.

Obviously this can't hurt though.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17 19:47         ` Daniel Jacobowitz
@ 2004-02-17 20:28           ` Elena Zannoni
  2004-02-17 20:44             ` Daniel Jacobowitz
  0 siblings, 1 reply; 8+ messages in thread
From: Elena Zannoni @ 2004-02-17 20:28 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

Daniel Jacobowitz writes:
 > On Tue, Feb 17, 2004 at 02:33:27PM -0500, Elena Zannoni wrote:
 > > Eli Zaretskii writes:
 > >  > > From: Elena Zannoni <ezannoni@redhat.com>
 > >  > > Date: Tue, 17 Feb 2004 09:30:06 -0500
 > >  > >  > >  
 > >  > >  > > -    MSYMBOL_TYPE (m) = mst_unknown;
 > >  > >  > 
 > >  > >  > This part of the patch seems to assume that mst_unknown has the value
 > >  > >  > zero.  Should we have such assumptions in the code?
 > >  > > 
 > >  > > Yes, in symtab.h mst_unknown is 0.
 > >  > 
 > >  > I know that, I looked it up in symtab.h.  The question is, should the
 > >  > code rely on the fact that zeroing out the struct causes the
 > >  > MSYMBOL_TYPE member to become mst_unknown.  Suppose we change the
 > >  > enumeration some day, that would break the code.
 > >  > 
 > > 
 > > Oh, I see. I misunderstood you. 
 > > 
 > >  > So I think a comment is not enough, we should leave that line alone.
 > >  > In general, it is my opinion that code should not assume anything
 > >  > about the numerical values of enumerated types.
 > > 
 > > That's true. It would be safer to leave the line alone. I'll change
 > > it back. Also language_unknown is used similarly.
 > > 
 > > I've committed this.
 > 
 > Thanks.  The only downside is that this suggests the type of the
 > terminating symbol can ever be read without being in error, which is
 > incorrect; the minimal symbol table's end appears to be marked by
 > otherwise by SYMBOL_LINKAGE_NAME (msym) == NULL.  Other minimal symbols
 > may have type mst_unknown.
 > 

I am not sure I follow your sentence. If you are worried that the
value 0 can indicate a perfectly legal msymbol, this is no different
from the situation we end up with when just using the memset. You can
always add a new enum value of mst_bogus (pick your name) and use that
in the terminating msym. BTW, there is almost identical code in
minsyms.c:install_minimal_symbols.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ob] Remove initializations of memset'd structure
  2004-02-17 20:28           ` Elena Zannoni
@ 2004-02-17 20:44             ` Daniel Jacobowitz
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Jacobowitz @ 2004-02-17 20:44 UTC (permalink / raw)
  To: gdb-patches

On Tue, Feb 17, 2004 at 03:24:09PM -0500, Elena Zannoni wrote:
> I am not sure I follow your sentence. If you are worried that the
> value 0 can indicate a perfectly legal msymbol, this is no different
> from the situation we end up with when just using the memset. You can
> always add a new enum value of mst_bogus (pick your name) and use that
> in the terminating msym. BTW, there is almost identical code in
> minsyms.c:install_minimal_symbols.

Nothing important.  My only point was that anything which read the type
of this minimal symbol was already off the end of the array, because
mst_unknown does not act as a terminator; everything should either use
minimal_symbol_count (most things do), or SYMBOL_LINKAGE_NAME (msym) ==
NULL.  So there is no need for the type to be mst_unknown (or anything
else).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2004-02-17 20:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-02-16 21:15 [ob] Remove initializations of memset'd structure Daniel Jacobowitz
2004-02-17  6:54 ` Eli Zaretskii
2004-02-17 14:34   ` Elena Zannoni
2004-02-17 19:12     ` Eli Zaretskii
2004-02-17 19:37       ` Elena Zannoni
2004-02-17 19:47         ` Daniel Jacobowitz
2004-02-17 20:28           ` Elena Zannoni
2004-02-17 20:44             ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox