Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #16472 > unrolled thread

possible NULL && dereferencing NULL pointer

Back to article view | Back to comp.lang.c

Contents

  possible NULL && dereferencing NULL pointer "Mark" <mark_cruzNOTFORSPAM@hotmail.com> - 2012-01-31 00:05 -0500
    Re: possible NULL && dereferencing NULL pointer Devil with the China Blue Dress <chine.bleu@yahoo.com> - 2012-01-30 21:39 -0800
      Re: possible NULL && dereferencing NULL pointer Jorgen Grahn <grahn+nntp@snipabacken.se> - 2012-01-31 08:25 +0000
      Re: possible NULL && dereferencing NULL pointer Keith Thompson <kst-u@mib.org> - 2012-01-31 00:28 -0800
        Re: possible NULL && dereferencing NULL pointer Devil with the China Blue Dress <chine.bleu@yahoo.com> - 2012-01-31 00:33 -0800
          Re: possible NULL && dereferencing NULL pointer Keith Thompson <kst-u@mib.org> - 2012-01-31 02:31 -0800
            Re: possible NULL && dereferencing NULL pointer Malcolm McLean <malcolm.mclean5@btinternet.com> - 2012-01-31 04:54 -0800
        Re: possible NULL && dereferencing NULL pointer gwowen <gwowen@gmail.com> - 2012-01-31 02:30 -0800
          Re: possible NULL && dereferencing NULL pointer Keith Thompson <kst-u@mib.org> - 2012-01-31 13:20 -0800
    Re: possible NULL && dereferencing NULL pointer Robert Wessel <robertwessel2@yahoo.com> - 2012-01-31 01:12 -0600
    Re: possible NULL && dereferencing NULL pointer Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-01-31 12:33 +0000
    Re: possible NULL && dereferencing NULL pointer jgk@panix.com (Joe keane) - 2012-02-01 15:15 +0000

#16472 — possible NULL && dereferencing NULL pointer

Hello

Consider the snippet (this is from the code base I have to maintain):

#define NAME_LEN 10
struct foo
{
  char  name[NAME_LEN];
  unsigned int time;
  /* more other fields */
};

struct master
{
  struct foo *f;
  unisgned int flags;
  struct hw_callbacks *hw_cb;
};

struct master *p_master = NULL;
...
/* allocate storage for struct master and assign ptr to p_master */
...

if (! p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))
{
   return ERR;
}

(and I have such construct all over the code)

I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
dereferencing NULL pointer), would it be more clear and correct to have:

if (! p_master->f)
   return ERR;

if (memcmp(p_master->f->name, "test", NAME_LEN))
  return ERR;

Thanks.

Mark 

[toc] | [next] | [standalone]

#16474

In article <jg7srm$ka8$1@speranza.aioe.org>,
 "Mark" <mark_cruzNOTFORSPAM@hotmail.com> wrote:

> I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
> dereferencing NULL pointer), would it be more clear and correct to have:

mem* and str* functions are generally undefined on null pointers. However it's 
easy enough to build your own function on top of these that do handle nulls:

    static inline int memorycmp(void *a, void *b, size_t n) {
        return a==b ? 0 : !a ? -1 : !b ? 1 : memcmp(a, b, n);
    }

extends memcmp to nulls, with the additional relations NULL==NULL and 
NULL<nonnull. On the other hand if you want NULL=="",

    static inline int memorycmp(void *a, void *b, size_t n) {
        if (!a) a = "";
        if (!b) b = "";
        return memcmp(a, b, n);
    }

You shouldn't be afraid of developping your own library on top of the std* 
libraries to adapt them to your own notion of rightness.

-- 
My name Indigo Montoya.      |         It's Coraline, not Caroline.
You flamed my father.        |       I'm whoever you want me to be.
Prepare to be spanked.       |  Annoying Usenet one post at a time.
Stop posting that!           |    At least I can stay in character.

[toc] | [prev] | [next] | [standalone]

#16477

On Tue, 2012-01-31, Devil with the China Blue Dress wrote:
> In article <jg7srm$ka8$1@speranza.aioe.org>,
>  "Mark" <mark_cruzNOTFORSPAM@hotmail.com> wrote:
>
>> I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
>> dereferencing NULL pointer), would it be more clear and correct to have:
>
> mem* and str* functions are generally undefined on null pointers. However it's 
> easy enough to build your own function on top of these that do handle nulls:
>
>     static inline int memorycmp(void *a, void *b, size_t n) {
>         return a==b ? 0 : !a ? -1 : !b ? 1 : memcmp(a, b, n);
>     }
>
> extends memcmp to nulls, with the additional relations NULL==NULL and 
> NULL<nonnull. On the other hand if you want NULL=="",
>
>     static inline int memorycmp(void *a, void *b, size_t n) {
>         if (!a) a = "";
>         if (!b) b = "";
>         return memcmp(a, b, n);
>     }
>
> You shouldn't be afraid of developping your own library on top of the std* 
> libraries to adapt them to your own notion of rightness.

I agree, but then IMHO you should (a) document the "notion of
rightness", e.g.

  /* In Foobar, it's normal for Baz to be NULL, so we
   * have helper functions which make it safe to compare a
   * Baz.name in that case, as if a NULL Baz had a "" name.
   */

And (b) choose a more narrow name than memorycmp(), which to me
implies memcmp(), which implies the semantics of memcmp().

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [next] | [standalone]

#16478

Devil with the China Blue Dress <chine.bleu@yahoo.com> writes:
> In article <jg7srm$ka8$1@speranza.aioe.org>,
>  "Mark" <mark_cruzNOTFORSPAM@hotmail.com> wrote:
>
>> I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
>> dereferencing NULL pointer), would it be more clear and correct to have:
>
> mem* and str* functions are generally undefined on null pointers. However it's 
> easy enough to build your own function on top of these that do handle nulls:
>
>     static inline int memorycmp(void *a, void *b, size_t n) {
>         return a==b ? 0 : !a ? -1 : !b ? 1 : memcmp(a, b, n);
>     }
>
> extends memcmp to nulls, with the additional relations NULL==NULL and 
> NULL<nonnull. On the other hand if you want NULL=="",
>
>     static inline int memorycmp(void *a, void *b, size_t n) {
>         if (!a) a = "";
>         if (!b) b = "";
>         return memcmp(a, b, n);
>     }
>
> You shouldn't be afraid of developping your own library on top of the std* 
> libraries to adapt them to your own notion of rightness.

Sure, but it's not clear to me that your memorycmp() is "right".

memcmp() compares the memory pointed to by one pointer to the memory
pointed to by another pointer.  A null pointer doesn't point to
anything, so I'd argue that if either pointer argument is null,
the comparison just doesn't make sense.  If you attempt to pass
one or two null pointers to memcmp(), it probably indicates a an
error in your program logic; hiding this error by quieting returning
a "sensible" value is not, IMHO, a good idea.

Mark, my advice is to fix your code's logic so it doesn't try to call
memcpy() with null arguments.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
    Will write code for food.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#16479

In article <lnliooxolh.fsf@nuthaus.mib.org>, Keith Thompson <kst-u@mib.org> 
wrote:

> Devil with the China Blue Dress <chine.bleu@yahoo.com> writes:
> > In article <jg7srm$ka8$1@speranza.aioe.org>,
> >  "Mark" <mark_cruzNOTFORSPAM@hotmail.com> wrote:
> >
> >> I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
> >> dereferencing NULL pointer), would it be more clear and correct to have:
> >
> > mem* and str* functions are generally undefined on null pointers. However 
> > it's 
> > easy enough to build your own function on top of these that do handle 
> > nulls:
> >
> >     static inline int memorycmp(void *a, void *b, size_t n) {
> >         return a==b ? 0 : !a ? -1 : !b ? 1 : memcmp(a, b, n);
> >     }
> >
> > extends memcmp to nulls, with the additional relations NULL==NULL and 
> > NULL<nonnull. On the other hand if you want NULL=="",
> >
> >     static inline int memorycmp(void *a, void *b, size_t n) {
> >         if (!a) a = "";
> >         if (!b) b = "";
> >         return memcmp(a, b, n);
> >     }
> >
> > You shouldn't be afraid of developping your own library on top of the std* 
> > libraries to adapt them to your own notion of rightness.
> 
> Sure, but it's not clear to me that your memorycmp() is "right".
> 
> memcmp() compares the memory pointed to by one pointer to the memory
> pointed to by another pointer.  A null pointer doesn't point to
> anything, so I'd argue that if either pointer argument is null,
> the comparison just doesn't make sense.  If you attempt to pass
> one or two null pointers to memcmp(), it probably indicates a an
> error in your program logic; hiding this error by quieting returning
> a "sensible" value is not, IMHO, a good idea.
> 
> Mark, my advice is to fix your code's logic so it doesn't try to call
> memcpy() with null arguments.

Ah. So you're advocating programming to someone else's libraries that are 
decades old rather than coding what your own intentions. Got it.

-- 
My name Indigo Montoya.      |         It's Coraline, not Caroline.
You flamed my father.        |       I'm whoever you want me to be.
Prepare to be spanked.       |  Annoying Usenet one post at a time.
Stop posting that!           |    At least I can stay in character.

[toc] | [prev] | [next] | [standalone]

#16480

Devil with the China Blue Dress <chine.bleu@yahoo.com> writes:
> In article <lnliooxolh.fsf@nuthaus.mib.org>, Keith Thompson <kst-u@mib.org> 
> wrote:
>> Devil with the China Blue Dress <chine.bleu@yahoo.com> writes:
[snip]
>> > You shouldn't be afraid of developping your own library on top of the std* 
>> > libraries to adapt them to your own notion of rightness.
>> 
>> Sure, but it's not clear to me that your memorycmp() is "right".
>> 
>> memcmp() compares the memory pointed to by one pointer to the memory
>> pointed to by another pointer.  A null pointer doesn't point to
>> anything, so I'd argue that if either pointer argument is null,
>> the comparison just doesn't make sense.  If you attempt to pass
>> one or two null pointers to memcmp(), it probably indicates a an
>> error in your program logic; hiding this error by quieting returning
>> a "sensible" value is not, IMHO, a good idea.
>> 
>> Mark, my advice is to fix your code's logic so it doesn't try to call
>> memcpy() with null arguments.
>
> Ah. So you're advocating programming to someone else's libraries that are 
> decades old rather than coding what your own intentions. Got it.

No.  I doubt that Mark's code, as posted, actually reflected his
intentions.

Here's the code in question:

    if (! p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))
    {
       return ERR;
    }

It calls memcmp() *only* if p_master->f is a null pointer.
That's almost certainly a bug.  It might not be there in Mark's
actual code, but it's in what he posted.

I advocate using the standard library when it's appropriate -- not
because it's obviously superior to any alternative, but because it's
standard.

Perhaps Mark would be better off developing his own library, but I see
no evidence of it so far.

Oh, and you might want to read the rest of what I wrote.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
    Will write code for food.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#16484

On Jan 31, 10:31 am, Keith Thompson <ks...@mib.org> wrote:
>
> Here's the code in question:
>
>     if (! p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))
>     {
>        return ERR;
>     }
>
> It calls memcmp() *only* if p_master->f is a null pointer.
> That's almost certainly a bug.  It might not be there in Mark's
> actual code, but it's in what he posted.
>
Yes, that's pretty obviously a bug.
The OP needs to think when p_master->f can be null and what that
means. If it means that the program has an internal error, there's
usually no point in masking it by not doing the memcmp. If it is a
vlid state for the pointer, he's got to decide whether that means a
match, a non-match, or something different.

[toc] | [prev] | [next] | [standalone]

#16481

On Jan 31, 8:28 am, Keith Thompson <ks...@mib.org> wrote:

> Mark, my advice is to fix your code's logic so it doesn't try to call
> memcpy() with null arguments.

But isn't a wrapper function a not-unreasonable way to do that?

I wouldn't recommend DwtCBD's particular level of clever-clever
obfuscation, but *at the very least* it changes undefined behaviour to
defined behaviour.

[toc] | [prev] | [next] | [standalone]

#16525

gwowen <gwowen@gmail.com> writes:
> On Jan 31, 8:28 am, Keith Thompson <ks...@mib.org> wrote:
>> Mark, my advice is to fix your code's logic so it doesn't try to call
>> memcpy() with null arguments.
>
> But isn't a wrapper function a not-unreasonable way to do that?
>
> I wouldn't recommend DwtCBD's particular level of clever-clever
> obfuscation, but *at the very least* it changes undefined behaviour to
> defined behaviour.

It's impossible to tell without more information.

Changing undefined behavior to defined behavior is not useful if the
defined behavior is logically incorrect.

A wrapper around memcpy() that checks for null pointer arguments and
does something reasonable *might* be a good idea.  But if the presence
of null pointers indicates a logical flaw in the code, then there is no
reasonable action to take (except perhaps to terminate the program with
an error message).  In that case, the correct solution would be to fix
the program so it doesn't encounter that situation in the first place.

Without more information from Mark, I don't think any further guessing
is particularly constructive.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
    Will write code for food.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#16475

On Tue, 31 Jan 2012 00:05:54 -0500, "Mark"
<mark_cruzNOTFORSPAM@hotmail.com> wrote:

>Hello
>
>Consider the snippet (this is from the code base I have to maintain):
>
>#define NAME_LEN 10
>struct foo
>{
>  char  name[NAME_LEN];
>  unsigned int time;
>  /* more other fields */
>};
>
>struct master
>{
>  struct foo *f;
>  unisgned int flags;
>  struct hw_callbacks *hw_cb;
>};
>
>struct master *p_master = NULL;
>...
>/* allocate storage for struct master and assign ptr to p_master */
>...
>
>if (! p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))
>{
>   return ERR;
>}
>
>(and I have such construct all over the code)
>
>I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
>dereferencing NULL pointer), would it be more clear and correct to have:
>
>if (! p_master->f)
>   return ERR;
>
>if (memcmp(p_master->f->name, "test", NAME_LEN))
>  return ERR;

Assuming you made a typo copying that, and it's actually:

 if (p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))

It's fine, and a fairly common idiom.  The && (and ||) operators are
defined to not evaluate ("short circuit") their second operands if the
first one has already determined the result).

If that's not a typo, you'll have problems since the *only* time you'd
execute the memcpy() is when you have a NULL pointer.

[toc] | [prev] | [next] | [standalone]

#16482

"Mark" <mark_cruzNOTFORSPAM@hotmail.com> writes:

> Consider the snippet (this is from the code base I have to maintain):
>
> #define NAME_LEN 10
> struct foo
> {
>   char  name[NAME_LEN];
>   unsigned int time;
>   /* more other fields */
> };
>
> struct master
> {
>   struct foo *f;
>   unisgned int flags;
>   struct hw_callbacks *hw_cb;
> };
>
> struct master *p_master = NULL;
> ...
> /* allocate storage for struct master and assign ptr to p_master */
> ...
>
> if (! p_master->f && memcmp (p_master->f->name, "test", NAME_LEN))
> {
>    return ERR;
> }
>
> (and I have such construct all over the code)

It's been suggested that this is a typo -- i.e. that the ! is there by
accident.  That's possible, but the code is wrong in other ways that
make it hard to know what was intended.

   memcmp(p_master->f->name, "test", NAME_LEN)

is undefined since "test" is not 10 bytes in size.  The posted code is
probably a simplification of the original, but the original might have
more clues about what the "if" is supposed to do.

> I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
> dereferencing NULL pointer), would it be more clear and correct to have:
>
> if (! p_master->f)
>    return ERR;
>
> if (memcmp(p_master->f->name, "test", NAME_LEN))
>   return ERR;

The original is wrong, but I can't say if this re-write is what is
intended.  What you've written here is equivalent to

  if (!p_master->f || memcmp(p_master->f->name, "test", 5))
     return ERR;

but returning ERR for all strings except "test" looks odd to me.

It's possible that the original author thought that the ! applied to the
whole expression.  I.e that the test should have been

  if (!(p_master->f && memcmp (p_master->f->name, "test", 5)))
      return ERR;

This is safe and returns ERR when either the pointer is null or the name
is the special value "test".

To avoid more guessing, what's the actual code?

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#16591

In article <jg7srm$ka8$1@speranza.aioe.org>,
Mark <mark_cruzNOTFORSPAM@hotmail.com> wrote:
>I'm not sure that memcmp() will not fail if p_master->bridge is NULL (i.e. 
>dereferencing NULL pointer), would it be more clear and correct to have:

In general, you don't pass NULL to a function that takes 'pointer to
so-and-so object', unless you have good evidence that it's allowed, and
it does what you want.

Otherwise, it will
    a) crash,
or
    b) do not what you want.

The 'strcmp' example is good.  What does a NULL argument mean?  Is it
the same thing as the string ""?  Or are NULL and "" two different
things?  Does NULL compare equal to "NULL"?

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web