[PATCH] jv-lang, guard against NULL

[PATCH] jv-lang, guard against NULL

[msnyder]

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: 255.txt --]
[-- Type: text/plain, Size: 955 bytes --]

2007-07-31  Michael Snyder  <msnyder@access-company.com>

	* jv-lang.c (java_link_class_type): Guard against NULL.

Index: jv-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-lang.c,v
retrieving revision 1.47
diff -p -r1.47 jv-lang.c
*** jv-lang.c	13 Jun 2007 17:30:01 -0000	1.47
--- jv-lang.c	31 Jul 2007 23:23:06 -0000
*************** java_link_class_type (struct type *type,
*** 407,414 ****
  	SET_TYPE_FIELD_PRIVATE (type, 0);
      }
  
!   i = strlen (name);
!   if (i > 2 && name[i - 1] == ']' && tsuper != NULL)
      {
        /* FIXME */
        TYPE_LENGTH (type) = TYPE_LENGTH (tsuper) + 4;	/* size with "length" */
--- 407,414 ----
  	SET_TYPE_FIELD_PRIVATE (type, 0);
      }
  
!   if (name != NULL && (i = strlen (name)) > 2 
!       && name[i - 1] == ']' && tsuper != NULL)
      {
        /* FIXME */
        TYPE_LENGTH (type) = TYPE_LENGTH (tsuper) + 4;	/* size with "length" */

Re: [PATCH] jv-lang, guard against NULL

[Daniel Jacobowitz]

On Tue, Jul 31, 2007 at 04:25:12PM -0700, Michael Snyder wrote:
> !   if (name != NULL && (i = strlen (name)) > 2 
> !       && name[i - 1] == ']' && tsuper != NULL)

Can we avoid assignments inside the if statement?  Also, there's an
unprotected call to strrchr above.  Maybe this should be an assert
instead, at the top of the function.

-- 
Daniel Jacobowitz
CodeSourcery

Re: FW: [PATCH] jv-lang, guard against NULL

[msnyder]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]


>
> On Tue, Jul 31, 2007 at 04:25:12PM -0700, Michael Snyder wrote:
>> !   if (name != NULL && (i = strlen (name)) > 2
>> !       && name[i - 1] == ']' && tsuper != NULL)
>
> Can we avoid assignments inside the if statement?

Yeah, I tried, but it ended up looking really awkward.
Test 'name', then do the strlen, then test name again.

> Also, there's an
> unprotected call to strrchr above.  Maybe this should be an assert
> instead, at the top of the function.

Good idea -- except, can we really be sure that name can
never legitimately be null?  Oh, yeah, I guess the unprotected
strrchr sort of tells us that, doesn't it?

OK, how about this?


[-- Attachment #2: 255b.txt --]
[-- Type: text/plain, Size: 1403 bytes --]

2007-07-31  Michael Snyder  <msnyder@access-company.com>

	* jv-lang.c (java_link_class_type): Guard against NULL.

Index: jv-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-lang.c,v
retrieving revision 1.47
diff -p -r1.47 jv-lang.c
*** jv-lang.c	13 Jun 2007 17:30:01 -0000	1.47
--- jv-lang.c	2 Aug 2007 21:29:07 -0000
*************** java_link_class_type (struct type *type,
*** 351,363 ****
    struct objfile *objfile = get_dynamics_objfile ();
    struct type *tsuper;
  
    unqualified_name = strrchr (name, '.');
    if (unqualified_name == NULL)
      unqualified_name = name;
  
    temp = clas;
    temp = value_struct_elt (&temp, NULL, "superclass", NULL, "structure");
!   if (name != NULL && strcmp (name, "java.lang.Object") == 0)
      {
        tsuper = get_java_object_type ();
        if (tsuper && TYPE_CODE (tsuper) == TYPE_CODE_PTR)
--- 351,364 ----
    struct objfile *objfile = get_dynamics_objfile ();
    struct type *tsuper;
  
+   gdb_assert (name != NULL);
    unqualified_name = strrchr (name, '.');
    if (unqualified_name == NULL)
      unqualified_name = name;
  
    temp = clas;
    temp = value_struct_elt (&temp, NULL, "superclass", NULL, "structure");
!   if (strcmp (name, "java.lang.Object") == 0)
      {
        tsuper = get_java_object_type ();
        if (tsuper && TYPE_CODE (tsuper) == TYPE_CODE_PTR)

Re: FW: [PATCH] jv-lang, guard against NULL

[Daniel Jacobowitz]

On Thu, Aug 02, 2007 at 02:30:01PM -0700, Michael Snyder wrote:
> Good idea -- except, can we really be sure that name can
> never legitimately be null?  Oh, yeah, I guess the unprotected
> strrchr sort of tells us that, doesn't it?
> 
> OK, how about this?

I've no complaint.  I don't think this sort of assert adds much value,
though.

-- 
Daniel Jacobowitz
CodeSourcery