Discussion:
My C program does not compile or gives segmentation fault
Add Reply
a***@gmail.com
2017-03-23 16:56:59 UTC
Reply
Permalink
Raw Message
Hi all I am trying to make a sample queue program. I just started with this concept and I am seeing a problem

#include <stdio.h>
#include <stdlib.h>
struct node {
int data;
struct node *next;
};

struct queue{
struct node *front;
struct node *rear;
};

struct queue *q=NULL;//pointer is initialised
q->front = NULL;//pointer members are initialised
q->rear = NULL;//initialised



int main(void)
{
struct node *ptr = NULL;

/*q->front = NULL;
q->rear = NULL;*/

ptr = (struct node *)malloc(sizeof(struct node));
if(ptr == NULL)
{
printf("error allocationg memory\n");
return 1;
}
ptr->next = NULL;
ptr->data = 10;
if(q->front == NULL)
{
q->front = ptr;
q->rear = ptr;
//rest of the logic
}
free(ptr);
return 0;
}

I am getting two problems,

1)If I create

q->front = NULL;
q->rear = NULL;

as global I get this warning

error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
q->front = NULL;
^
queue.c:15:2: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
q->rear = NULL;

2) If I create same variables inside main, I can compile, but at runtime I am getting segmentation fault.
Why is this happening?
my setup is **gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3)**
maybe this is a stupid question.
GOTHIER Nathan
2017-03-23 17:04:02 UTC
Reply
Permalink
Raw Message
On Thu, 23 Mar 2017 09:56:59 -0700 (PDT)
Post by a***@gmail.com
Hi all I am trying to make a sample queue program. I just started with this concept and I am seeing a problem
#include <stdio.h>
#include <stdlib.h>
struct node {
int data;
struct node *next;
};
struct queue{
struct node *front;
struct node *rear;
};
struct queue *q=NULL;//pointer is initialised
Here you declare and initialize a pointer (i.e. a memory address)...
Post by a***@gmail.com
q->front = NULL;//pointer members are initialised
q->rear = NULL;//initialised
But you can't initialize objects that aren't allocated since your pointer
points to the null memory address where by convention there is no object.
Richard Heathfield
2017-03-23 17:07:44 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Hi all I am trying to make a sample queue program. I just started with this concept and I am seeing a problem
#include <stdio.h>
#include <stdlib.h>
struct node {
int data;
struct node *next;
};
struct queue{
struct node *front;
struct node *rear;
};
struct queue *q=NULL;//pointer is initialised
Yes, this is an initialisation of a file scope object, so it's legal.
Post by a***@gmail.com
q->front = NULL;//pointer members are initialised
No, this is not an initialisation. This is an assignment statement.
Since it appears outside a function body, a diagnostic message is
required. Fix: put it in main(). But that won't solve all your problems,
because q is NULL, so what you are trying to do here is set NULL->front
= NULL, which is never going to work.
Post by a***@gmail.com
q->rear = NULL;//initialised
Same problems.
Post by a***@gmail.com
ptr = (struct node *)malloc(sizeof(struct node));
Much cleaner:

ptr = malloc(sizeof *ptr);
Post by a***@gmail.com
I am getting two problems,
1)If I create
q->front = NULL;
q->rear = NULL;
as global I get this warning
error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
q->front = NULL;
It's complaining here about your first problem - an assignment statement
outside a function.
Post by a***@gmail.com
2) If I create same variables inside main, I can compile, but at runtime I am getting segmentation fault.
That's your second problem: assigning a value to NULL->front.

Fix: wait until q points somewhere valid before trying to set its front
and rear pointers. Until q does point somewhere valid, q->front and
q->rear are meaningless.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
a***@gmail.com
2017-03-23 17:36:24 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by a***@gmail.com
Hi all I am trying to make a sample queue program. I just started with this concept and I am seeing a problem
#include <stdio.h>
#include <stdlib.h>
struct node {
int data;
struct node *next;
};
struct queue{
struct node *front;
struct node *rear;
};
struct queue *q=NULL;//pointer is initialised
Yes, this is an initialisation of a file scope object, so it's legal.
Post by a***@gmail.com
q->front = NULL;//pointer members are initialised
No, this is not an initialisation. This is an assignment statement.
Since it appears outside a function body, a diagnostic message is
required. Fix: put it in main(). But that won't solve all your problems,
because q is NULL, so what you are trying to do here is set NULL->front
= NULL, which is never going to work.
Post by a***@gmail.com
q->rear = NULL;//initialised
Same problems.
Post by a***@gmail.com
ptr = (struct node *)malloc(sizeof(struct node));
ptr = malloc(sizeof *ptr);
Post by a***@gmail.com
I am getting two problems,
1)If I create
q->front = NULL;
q->rear = NULL;
as global I get this warning
error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
q->front = NULL;
It's complaining here about your first problem - an assignment statement
outside a function.
Post by a***@gmail.com
2) If I create same variables inside main, I can compile, but at runtime I am getting segmentation fault.
That's your second problem: assigning a value to NULL->front.
Fix: wait until q points somewhere valid before trying to set its front
and rear pointers. Until q does point somewhere valid, q->front and
q->rear are meaningless.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
I see example codes where q is never initialised to NULL and a function is there that says 'createq'. All it do in the function is just make rear and front to point to NULL. At that moment nothing is allocated to anyone.
a***@gmail.com
2017-03-23 17:42:14 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by a***@gmail.com
Hi all I am trying to make a sample queue program. I just started with this concept and I am seeing a problem
#include <stdio.h>
#include <stdlib.h>
struct node {
int data;
struct node *next;
};
struct queue{
struct node *front;
struct node *rear;
};
struct queue *q=NULL;//pointer is initialised
Yes, this is an initialisation of a file scope object, so it's legal.
Post by a***@gmail.com
q->front = NULL;//pointer members are initialised
No, this is not an initialisation. This is an assignment statement.
Since it appears outside a function body, a diagnostic message is
required. Fix: put it in main(). But that won't solve all your problems,
because q is NULL, so what you are trying to do here is set NULL->front
= NULL, which is never going to work.
Post by a***@gmail.com
q->rear = NULL;//initialised
Same problems.
Post by a***@gmail.com
ptr = (struct node *)malloc(sizeof(struct node));
ptr = malloc(sizeof *ptr);
Post by a***@gmail.com
I am getting two problems,
1)If I create
q->front = NULL;
q->rear = NULL;
as global I get this warning
error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
q->front = NULL;
It's complaining here about your first problem - an assignment statement
outside a function.
Post by a***@gmail.com
2) If I create same variables inside main, I can compile, but at runtime I am getting segmentation fault.
That's your second problem: assigning a value to NULL->front.
Fix: wait until q points somewhere valid before trying to set its front
and rear pointers. Until q does point somewhere valid, q->front and
q->rear are meaningless.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
I see example codes where q is never initialised to NULL and a function is there that says 'createq'. All it do in the function is just make rear and front to point to NULL. At that moment nothing is allocated to anyone. Like this
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}

So what does that mean?
GOTHIER Nathan
2017-03-23 17:52:30 UTC
Reply
Permalink
Raw Message
On Thu, 23 Mar 2017 10:42:14 -0700 (PDT)
Post by a***@gmail.com
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}
So what does that mean?
It means you should rather learn C before asking trivial questions. ;-)
Joe Pfeiffer
2017-03-23 18:54:34 UTC
Reply
Permalink
Raw Message
Post by GOTHIER Nathan
On Thu, 23 Mar 2017 10:42:14 -0700 (PDT)
Post by a***@gmail.com
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}
So what does that mean?
It means you should rather learn C before asking trivial questions. ;-)
If you can't be helpful, please be quiet.
GOTHIER Nathan
2017-03-23 23:02:33 UTC
Reply
Permalink
Raw Message
On Thu, 23 Mar 2017 12:54:34 -0600
Post by Joe Pfeiffer
If you can't be helpful, please be quiet.
I'm afraid you're confusing me with one of your students... so just mind your
own business the class is over!
Joe Pfeiffer
2017-03-23 23:16:49 UTC
Reply
Permalink
Raw Message
Post by GOTHIER Nathan
On Thu, 23 Mar 2017 12:54:34 -0600
Post by Joe Pfeiffer
If you can't be helpful, please be quiet.
I'm afraid you're confusing me with one of your students... so just mind your
own business the class is over!
No, I was confusing you with someone who might possibly stop being an
asshole if it were pointed out to him. I'll not make that mistake
again.
GOTHIER Nathan
2017-03-23 23:32:44 UTC
Reply
Permalink
Raw Message
On Thu, 23 Mar 2017 17:16:49 -0600
Post by Joe Pfeiffer
No, I was confusing you with someone who might possibly stop being an
asshole if it were pointed out to him. I'll not make that mistake
again.
After so many years in the academic field you only learned insults... you're
pathetic old man! It's sad nobody will regret you after you passed away.
Joe Pfeiffer
2017-03-23 17:57:20 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone. Like this
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}
This isn't the whole program, of course. What does the complete program
look like?
a***@gmail.com
2017-03-23 18:23:13 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
Post by a***@gmail.com
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone. Like this
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}
This isn't the whole program, of course. What does the complete program
look like?
Also I came across this code on another website
http://quiz.geeksforgeeks.org/queue-set-2-linked-list-implementation/
Joe Pfeiffer
2017-03-23 18:53:58 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone. Like this
int main()
{
create_queue(q);//
//some code
}
create_queue(struct queue* q)
{
q -> rear = NULL;
q -> front = NULL;
}
This isn't the whole program, of course. What does the complete program
look like?
Also I came across this code on another website
http://quiz.geeksforgeeks.org/queue-set-2-linked-list-implementation/
In that code, the declaration of q includes an initialization:

struct Queue *q = createQueue();

so it calls createQueue() from within main().

Within createQueue(), we have:

struct Queue *createQueue()
{
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
q->front = q->rear = NULL;
return q;
}

The call to malloc() reserves space for q, and points it at that space.
The function then returns the allocated space to the main program.

(Two things I noticed that aren't actually relevant to your question,
but I'll mention:

Many people would prefer to see

struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));

replaced with

struct Queue *q = malloc(sizeof *q);

on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof. Others regard the
longer version as being a bit of self-documentation.

Also, it's a good idea to check the return value from malloc(); if the
call fails then the the value returned by malloc() will be NULL)
Richard Heathfield
2017-03-23 19:00:58 UTC
Reply
Permalink
Raw Message
On 23/03/17 18:53, Joe Pfeiffer wrote:
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
Post by Joe Pfeiffer
Also, it's a good idea to check the return value from malloc(); if the
call fails then the the value returned by malloc() will be NULL)
Right.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-23 19:59:49 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard

See recommendation MEM02-C (Memory Management):
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-23 20:40:00 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
David Brown
2017-03-23 21:06:55 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
A key point in the example on the cert site is that there is a distance
between the pointer definition, and the assignment to it:

widget *p;

/* ... */

p = malloc(sizeof(gadget));

In code like that, the risk of getting the malloc wrong is much higher,
and thus the benefits of putting in the cast are much higher, compared to:

widget *p = malloc(sizeof(gadget));

or

widget *p = malloc(sizeof *p);


IMHO, a key point is to define the pointer variable only when you need
it - make the malloc an initialisation, not an assignment. That gives
you "self-documenting code", with the type written only once, and shows
the reader exactly what is happening, unlike

*p = malloc(sizeof *p);
Jerry Stuckle
2017-03-24 00:43:09 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Ian Collins
2017-03-24 01:09:06 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
The authour agrees with the consensus here in a later comment:

"Much earlier, John Bode suggested the following compliant solution:
widget *p;
...
p = malloc(sizeof *p);

as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
--
Ian
Jerry Stuckle
2017-03-24 01:28:47 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Ian Collins
2017-03-24 01:33:33 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
So you claim Robert Seacord is contradicting himself?
--
Ian
Jerry Stuckle
2017-03-24 13:23:00 UTC
Reply
Permalink
Raw Message
Post by Ian Collins
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
So you claim Robert Seacord is contradicting himself?
Nope. But I already know you don't read so well.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Ian Collins
2017-03-24 19:39:52 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Ian Collins
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
So you claim Robert Seacord is contradicting himself?
Nope. But I already know you don't read so well.
Did you fail to read "John Bode suggested the following compliant solution"?
--
Ian
Jerry Stuckle
2017-03-25 00:40:53 UTC
Reply
Permalink
Raw Message
Post by Ian Collins
Post by Jerry Stuckle
Post by Ian Collins
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT
discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
So you claim Robert Seacord is contradicting himself?
Nope. But I already know you don't read so well.
Did you fail to read "John Bode suggested the following compliant solution"?
I read it. Too bad you don't understand what it's saying. But that's
normal for you, Ian.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Joe Pfeiffer
2017-03-24 01:48:01 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by David Brown
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
Ummm, he said it was "... a reasonable *compliant* solution that would
*seem* to be noncompliant given the title..." (emphasis added)

Hence my remark elsewhere that the rule (and its title) needs to be
modified so we don't have to wade through the comments to discover that
the near-consensus here is also compliant.
John Bode
2017-03-24 20:32:34 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
T *p = malloc( sizeof *p );

*always* does the right thing. There's no chance of a type or size
mismatch. There's no chance of accidentally allocating the wrong amount
of memory. This idiom will always work everywhere in C. It won't work in
C++, but if you're writing C++ you *shouldn't use malloc()*.

Remember when malloc() returned char * instead of void *? The necessary
casting gymnastics were a *pain in the ass* and caused more problems than
they solved. That's part of why WG14 introduced the void * type IN THE
FIRST PLACE.

The CERT recommendation (and that's all it is, a recommendation) is flawed.
The rule *should* be to always use "sizeof *<target pointer>".

Besides, as of C99 you should *never* declare a pointer to dynamic memory
until you're ready to allocate that memory. Instead of writing

T *p;

// many lines of code

p = malloc( sizeof *p );

you should write

// many lines of code

T *p = malloc( sizeof *p );
Jerry Stuckle
2017-03-25 00:42:34 UTC
Reply
Permalink
Raw Message
Post by John Bode
Post by Jerry Stuckle
Post by David Brown
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion is
probably orthogonal to this guideline, but the approach of taking the
sizeof *p seems to be a reasonable compliant solution that would seem to
be noncompliant given the title of this rule."
Which is *noncomplient*. Reasonable or not.
T *p = malloc( sizeof *p );
*always* does the right thing. There's no chance of a type or size
mismatch. There's no chance of accidentally allocating the wrong amount
of memory. This idiom will always work everywhere in C. It won't work in
C++, but if you're writing C++ you *shouldn't use malloc()*.
Remember when malloc() returned char * instead of void *? The necessary
casting gymnastics were a *pain in the ass* and caused more problems than
they solved. That's part of why WG14 introduced the void * type IN THE
FIRST PLACE.
The CERT recommendation (and that's all it is, a recommendation) is flawed.
The rule *should* be to always use "sizeof *<target pointer>".
Besides, as of C99 you should *never* declare a pointer to dynamic memory
until you're ready to allocate that memory. Instead of writing
T *p;
// many lines of code
p = malloc( sizeof *p );
you should write
// many lines of code
T *p = malloc( sizeof *p );
YOU say the CERT recommendation is flawed. C experts around the world
disagree with you.

And you don't always want to allocate memory at the time you declare the
pointer. In fact, you may not allocate memory at all. But you may need
to declare the pointer earlier for scoping reasons.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Joe Pfeiffer
2017-03-24 01:29:15 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by Jerry Stuckle
So you know more than CERT. ROFLMAO!
widget *p;
...
p = malloc(sizeof *p);
as well as the use of Abstract Data Types (ADT). The ADT discussion
is probably orthogonal to this guideline, but the approach of taking
the sizeof *p seems to be a reasonable compliant solution that would
seem to be noncompliant given the title of this rule."
Ah, so I just hadn't read the comments sufficiently, He really ought to
(at the very least) point out that solution is another compliant option
without making people read the comments.
Richard Heathfield
2017-03-24 10:10:34 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.

As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.

If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 13:26:27 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.

I would say they know more than the idiots in this newsgroup.

But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 13:52:14 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
grounds. Let me summarise for you:

1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.

Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 14:30:30 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds. They are a matter of style. And style
only.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 14:37:44 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds.
I disagree. A random dictionary search for "technical" reveals these two
definitions (no doubt there are more): "relating to a particular
subject, art, craft, or its techniques"; "involving or concerned with
applied and industrial sciences".

My argument certainly qualifies by the first definition. If you disagree
with /that/, take it up in an English usage newsgroup.
Post by Jerry Stuckle
They are a matter of style. And style only.
But that's *not* a technical argument. Not that I was expecting one.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 16:51:48 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as
mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds.
I disagree. A random dictionary search for "technical" reveals these two
definitions (no doubt there are more): "relating to a particular
subject, art, craft, or its techniques"; "involving or concerned with
applied and industrial sciences".
ROFLMAO! That doesn't make your argument "technical" - except in your mind.
Post by Richard Heathfield
My argument certainly qualifies by the first definition. If you disagree
with /that/, take it up in an English usage newsgroup.
Post by Jerry Stuckle
They are a matter of style. And style only.
But that's *not* a technical argument. Not that I was expecting one.
Nope. I don't argue styles under the guise of "technical".

Face it. You just can't stand that someone more intelligent and better
at programming C than you disagrees.

Well, here's too words for you: tough shit.

CERT standards are widely recognized as good programming practices.
Richard's standards are recognized by no one except a few in this group.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 18:05:06 UTC
Reply
Permalink
Raw Message
<snip>
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
They are a matter of style. And style only.
But that's *not* a technical argument. Not that I was expecting one.
Nope. I don't argue styles under the guise of "technical".
Face it. You just can't stand that someone more intelligent and better
at programming C than you disagrees.
Well, here's too words for you: tough shit.
CERT standards are widely recognized as good programming practices.
Richard's standards are recognized by no one except a few in this group.
Again, that's an appeal to authority that does not demonstrate any
knowledge of the C language. You have had ample opportunity to provide
technical arguments to support your view and have refused every time.

I am now getting people complaining at me in email for feeding the
troll, so I'm going to leave it there until such time as you come up
with something a bit more meaty than "Because a C Programmer at an
American University Says So".
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 19:09:48 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
<snip>
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
They are a matter of style. And style only.
But that's *not* a technical argument. Not that I was expecting one.
Nope. I don't argue styles under the guise of "technical".
Face it. You just can't stand that someone more intelligent and better
at programming C than you disagrees.
Well, here's too words for you: tough shit.
CERT standards are widely recognized as good programming practices.
Richard's standards are recognized by no one except a few in this group.
Again, that's an appeal to authority that does not demonstrate any
knowledge of the C language. You have had ample opportunity to provide
technical arguments to support your view and have refused every time.
I am now getting people complaining at me in email for feeding the
troll, so I'm going to leave it there until such time as you come up
with something a bit more meaty than "Because a C Programmer at an
American University Says So".
It is an authority which is widely recognized around the world.

And when you come up with a technical argument against it, I will be
quite happy to discuss it. But you still have not done so.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Jerry Stuckle
2017-03-24 16:55:20 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as
mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds.
I disagree. A random dictionary search for "technical" reveals these two
definitions (no doubt there are more): "relating to a particular
subject, art, craft, or its techniques"; "involving or concerned with
applied and industrial sciences".
My argument certainly qualifies by the first definition. If you disagree
with /that/, take it up in an English usage newsgroup.
Post by Jerry Stuckle
They are a matter of style. And style only.
But that's *not* a technical argument. Not that I was expecting one.
Oh, and BTW - your definition of technical is nothing more than a quote
of an authority. So according to you, it is meaningless.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
j***@verizon.net
2017-03-24 16:45:36 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds. They are a matter of style. And style
only.
Well, no one's claiming that it's not legal to use the unnecessarily obfuscated approach, or that it won't work. We're only arguing that it's a bad idea, and there are sound technical reasons why that's the case.. The second of Richard's arguments is about not hiding a bug, which counts as a technical ground to me, though I'm sure you and I will never agree on the definition of "technical". CERT themselves found that reason sufficiently persuasive to make an exception for code that needs to compile under C90.

The most fundamental reason why

p = malloc(count * sizeof *p);

is better than

p = (int*)malloc(count * sizeof(int));

is that the more obfuscated approach serves only to solve a problem of it's own creation. The type doesn't matter with malloc (which is the key way that it differs from C++'s "new" operator). The memory allocated by malloc() is guaranteed to be correctly aligned for any type, and the return type is void* so it can implicitly be converted to any desired type.

The key thing that does matter to malloc() is the size, not the type. The expression "count *sizeof *p" manifestly gives the right size, as you can easily verify just by looking at that one line of code, without even needing to know the type of p. "count * sizeof(int)" might be the right size, if p is defined as int*, but the only way to be sure is to locate the declaration of p. That declaration might be on the same line, but it could also be on another line far away, particularly if it's a member of struct that's declared in a header file.

The obfuscated approach deals with this problem (which is of it's own creation, since "count * sizeof *p" doesn't have that problem in the first place), by requiring that a matching cast be made to malloc()'s return value, that performs precisely the same conversion that would have occurred anyway, so as to ensure that a diagnostic message will be mandatory if the types aren't compatible.

Note, in particular, that the obfuscated approach creates a breading ground for certain types of errors. For instance, when updating code, a careless coder (and there's plenty of those to go around) could update one of the two types, without updating the other:

p = (int_least64_t*)malloc(count * sizeof(int_least32_t));

The compiler would provide no help at catching this error, assuming that int_least64_t* is the correct type for p. Since the things that don't match are on the same line, it's pretty easy to catch such errors, but despite that fact I have seen precisely this type of mistake committed a couple of time.

The better approach has an analogous failure mode - if the name of a variable changes, a careless coder could leave you with

q = malloc(count * sizeof *p);

However, changes in the name of a variable generally (in my experience) mean that the old variable name is no longer in use, so the compiler will typically be able to point out such errors to you.

I'm not posting this in the expectation of being able to convince you - that would be a ridiculous thing to expect of you. I'm also not expecting this to prompt a discussion of the technical merits of the two approaches, that would be an equally ridiculous expectation. I'm posting for the benefit of more open-minded individuals who might be reading this, because it makes arguments that are somewhat distinct from Richard's.
Jerry Stuckle
2017-03-24 16:52:40 UTC
Reply
Permalink
Raw Message
Post by j***@verizon.net
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people.
That's not a technical argument. By the way, Robert Seacord came /here/
seeking consensus, and he didn't get it. Several people here put a lot
of work into pointing out many of the flaws in the CERT document. You
will find those people's names in the Acknowledgements section, but you
will not find their corrections in the text because, for the most part,
their views were ignored.
Post by Jerry Stuckle
And a large number of other
C experts have known over the years agree with CERT.
Again, that's not a technical argument.
Post by Jerry Stuckle
I would say they know more than the idiots in this newsgroup.
Nor is that.
Post by Jerry Stuckle
But you can't argue the case on its merits, so you have to just dismiss
the source. That's because you can't argue your case on technical grounds.
Projection. I have already argued this case many times on technical
1) all things being equal, simpler code is better;
2) the cast is unnecessary (and, in C90, could hide a bug, that of
failing to #include <stdlib.h>);
3) the code is more robust in the face of type changes.
Now, do you have any *technical* arguments to present, or are you going
to stick to your unconvincing appeals to dubious authority?
Those are not technical grounds. They are a matter of style. And style
only.
Well, no one's claiming that it's not legal to use the unnecessarily obfuscated approach, or that it won't work. We're only arguing that it's a bad idea, and there are sound technical reasons why that's the case.. The second of Richard's arguments is about not hiding a bug, which counts as a technical ground to me, though I'm sure you and I will never agree on the definition of "technical". CERT themselves found that reason sufficiently persuasive to make an exception for code that needs to compile under C90.
The most fundamental reason why
p = malloc(count * sizeof *p);
is better than
p = (int*)malloc(count * sizeof(int));
is that the more obfuscated approach serves only to solve a problem of it's own creation. The type doesn't matter with malloc (which is the key way that it differs from C++'s "new" operator). The memory allocated by malloc() is guaranteed to be correctly aligned for any type, and the return type is void* so it can implicitly be converted to any desired type.
The key thing that does matter to malloc() is the size, not the type. The expression "count *sizeof *p" manifestly gives the right size, as you can easily verify just by looking at that one line of code, without even needing to know the type of p. "count * sizeof(int)" might be the right size, if p is defined as int*, but the only way to be sure is to locate the declaration of p. That declaration might be on the same line, but it could also be on another line far away, particularly if it's a member of struct that's declared in a header file.
The obfuscated approach deals with this problem (which is of it's own creation, since "count * sizeof *p" doesn't have that problem in the first place), by requiring that a matching cast be made to malloc()'s return value, that performs precisely the same conversion that would have occurred anyway, so as to ensure that a diagnostic message will be mandatory if the types aren't compatible.
p = (int_least64_t*)malloc(count * sizeof(int_least32_t));
The compiler would provide no help at catching this error, assuming that int_least64_t* is the correct type for p. Since the things that don't match are on the same line, it's pretty easy to catch such errors, but despite that fact I have seen precisely this type of mistake committed a couple of time.
The better approach has an analogous failure mode - if the name of a variable changes, a careless coder could leave you with
q = malloc(count * sizeof *p);
However, changes in the name of a variable generally (in my experience) mean that the old variable name is no longer in use, so the compiler will typically be able to point out such errors to you.
I'm not posting this in the expectation of being able to convince you - that would be a ridiculous thing to expect of you. I'm also not expecting this to prompt a discussion of the technical merits of the two approaches, that would be an equally ridiculous expectation. I'm posting for the benefit of more open-minded individuals who might be reading this, because it makes arguments that are somewhat distinct from Richard's.
And CERT, a highly respected organization, has standards which disagree
with you.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
j***@verizon.net
2017-03-24 17:10:47 UTC
Reply
Permalink
Raw Message
On Friday, March 24, 2017 at 12:52:38 PM UTC-4, Jerry Stuckle wrote:
...
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
That's not news. Most standards have defects (ISO C definitely included). In
fact, I'd be hard pressed to believe that any standard was perfect. For example,
every requirement from MISRA that's been mentioned in this group has been, at
best, poorly motivated, and at worst, positively wrong headed. I don't own a
copy, so I can't be sure, but I expect it contains a great many reasonable
requirements - but it's the others that are disproportionately likely to be
discussed here.

That's why I'm only interested in the technical reasons you will never be able
to give for this preference, not the argument from authority which is the only
thing you ever bother to give.

I read the explanation CERT gave for their requirement, and that was based
entirely upon the unstated but incorrect assumption that the type, rather than
the size, was important.
David Brown
2017-03-24 17:48:42 UTC
Reply
Permalink
Raw Message
Post by j***@verizon.net
...
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
That's not news. Most standards have defects (ISO C definitely included). In
fact, I'd be hard pressed to believe that any standard was perfect. For example,
every requirement from MISRA that's been mentioned in this group has been, at
best, poorly motivated, and at worst, positively wrong headed. I don't own a
copy, so I can't be sure, but I expect it contains a great many reasonable
requirements - but it's the others that are disproportionately likely to be
discussed here.
I do have a copy of MISRA, and I'd guess very roughly that 60% of the
rules are quite good ideas, 20% are not particularly helpful, and 20%
are downright bad. Many of the good ones are pretty obvious (like "do
not rely on undefined behaviour"). Many of the ones I consider bad had
are motivated by poor tools - a developer (especially for the kinds of
software where MISRA is appropriate) should get better tools rather than
write less clear code. And some are a matter of style and opinion.
Post by j***@verizon.net
That's why I'm only interested in the technical reasons you will never be able
to give for this preference, not the argument from authority which is the only
thing you ever bother to give.
I read the explanation CERT gave for their requirement, and that was based
entirely upon the unstated but incorrect assumption that the type, rather than
the size, was important.
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.

In my opinion, the problem does not lie in casting or not casting the
result of malloc - it lies in the separation of the definition of "p"
from its assignment. And the solution is to write your mallocs in the
form "widget *p = malloc(sizeof *p);" - then there is no doubt about
your types, your sizes, or what you are actually dealing with at that
point in the code.

And then you can safely cast your pointer if you want C++ compatibility
- the risk of introducing a mistake here is very small. People who let
their compilers use implicit declarations of functions without proper
#includes and without warnings should find themselves a new career.

You can also write "sizeof(widget)" without worrying about surprises if
types change, because you have no choice but to change it on that line
anyway. That saves the risk of the typo "malloc(sizeof p)".
James Kuyper
2017-03-24 18:04:05 UTC
Reply
Permalink
Raw Message
On 03/24/2017 01:48 PM, David Brown wrote:
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.
Yes, but "sizeof *p" avoids that problem perfectly, without making any
mention of the type.
Post by David Brown
In my opinion, the problem does not lie in casting or not casting the
result of malloc - it lies in the separation of the definition of "p"
from its assignment. And the solution is to write your mallocs in the
form "widget *p = malloc(sizeof *p);" - then there is no doubt about
your types, your sizes, or what you are actually dealing with at that
point in the code.
The minute you're talking about variables which are members of structs
(a VERY common case), it's generally not reasonable to put the
definition and the assignment in the same place.

...
Post by David Brown
You can also write "sizeof(widget)" without worrying about surprises if
types change, because you have no choice but to change it on that line
anyway. ...
"no choice"?? I've seen people make precisely that choice, and
obviously, the compiler didn't object. It did cause problems during
testing, of course (but only if the incorrect size was too small).
David Brown
2017-03-24 18:16:31 UTC
Reply
Permalink
Raw Message
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.
Yes, but "sizeof *p" avoids that problem perfectly, without making any
mention of the type.
No, it makes this bug even harder to find.

If the user writes "p = malloc(sizeof gadget)" when p is declared as
"widget *", then you can see that they are mixing up gadgets and
widgets. If the user, with the same logical mixup, writes "p =
malloc(*p)", then it is harder to spot the logical flaw.

The point is, there is a bug somewhere. If you are lucky, the compiler
can help find it. Less lucky, you can see it from the code.

I am not saying that writing "malloc(sizeof gadget)" is necessarily a
good idea overall - but it may have some merits.
Post by James Kuyper
Post by David Brown
In my opinion, the problem does not lie in casting or not casting the
result of malloc - it lies in the separation of the definition of "p"
from its assignment. And the solution is to write your mallocs in the
form "widget *p = malloc(sizeof *p);" - then there is no doubt about
your types, your sizes, or what you are actually dealing with at that
point in the code.
The minute you're talking about variables which are members of structs
(a VERY common case), it's generally not reasonable to put the
definition and the assignment in the same place.
Agreed, that makes things a little different.

However, it may well make sense to write:

widget *p = malloc(sizeof *p);
if (p) {
s.widget_ptr = p;
// ...
} else {
panic();
}

The "best" choice can depend on the code in question.

And as has also been mentioned, factory functions or creator functions
of some kind could easily be a better solution.
Post by James Kuyper
...
Post by David Brown
You can also write "sizeof(widget)" without worrying about surprises if
types change, because you have no choice but to change it on that line
anyway. ...
"no choice"?? I've seen people make precisely that choice, and
obviously, the compiler didn't object. It did cause problems during
testing, of course (but only if the incorrect size was too small).
Fair enough - people can always find ways to choose to write incorrect code!
James Kuyper
2017-03-24 19:10:19 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
I just realized that your example is a syntax error unless gadget is the
name of a variable. I had thought it was meant to be a typedef name. Had
you intended to write

p = malloc(sizeof(gadget));
Post by David Brown
Post by James Kuyper
Post by David Brown
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.
Yes, but "sizeof *p" avoids that problem perfectly, without making any
mention of the type.
No, it makes this bug even harder to find.
If the user writes "p = malloc(sizeof gadget)" when p is declared as
"widget *", then you can see that they are mixing up gadgets and
widgets. If the user, with the same logical mixup, writes "p =
malloc(*p)", then it is harder to spot the logical flaw.
The only thing wrong with

p = malloc(*p);

is the missing sizeof operator. It would seem to me that the analogous
error with the other approach would be:

p = (widget*)malloc(widget);

I have to agree - malloc(widget) is always a syntax error, while
malloc(*p) is only a syntax error is *p doesn't have arithmetic type, so
that's a very minor argument in favor of using the type rather than the
expression, depending upon how likely you are to forget the "sizeof"
operator.

I don't see how the "same logical mixup" could lead to both

p = malloc(sizeof gadget);

and

p = malloc(*p);

The first comes from forgetting the type of *p, an error that's not
particular unusual in code that has a large number of user-defined
types. The second comes from forgetting to type sizeof, a very different
type of error, and one I would not describe as a "logical mixup".
James Kuyper
2017-03-24 19:17:15 UTC
Reply
Permalink
Raw Message
On 03/24/2017 03:10 PM, James Kuyper wrote:
...
Post by James Kuyper
malloc(*p) is only a syntax error is *p doesn't have arithmetic type, so
That should have been "constraint violation", not "syntax error".
David Brown
2017-03-24 23:49:45 UTC
Reply
Permalink
Raw Message
Post by James Kuyper
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
I just realized that your example is a syntax error unless gadget is the
name of a variable. I had thought it was meant to be a typedef name. Had
you intended to write
p = malloc(sizeof(gadget));
Yes, that was my intention. Sorry for the mistake.
Post by James Kuyper
Post by David Brown
Post by James Kuyper
Post by David Brown
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.
Yes, but "sizeof *p" avoids that problem perfectly, without making any
mention of the type.
No, it makes this bug even harder to find.
If the user writes "p = malloc(sizeof gadget)" when p is declared as
"widget *", then you can see that they are mixing up gadgets and
widgets. If the user, with the same logical mixup, writes "p =
malloc(*p)", then it is harder to spot the logical flaw.
The only thing wrong with
p = malloc(*p);
is the missing sizeof operator.
Another silly mistake from me. I must have made that post in a rush.

p = malloc(sizeof *p);

is not /wrong/. I just wonder if it would make it harder to spot a
logical flaw in the program if the user has got his types mixed up.
Post by James Kuyper
It would seem to me that the analogous
p = (widget*)malloc(widget);
I have to agree - malloc(widget) is always a syntax error, while
malloc(*p) is only a syntax error is *p doesn't have arithmetic type, so
that's a very minor argument in favor of using the type rather than the
expression, depending upon how likely you are to forget the "sizeof"
operator.
I don't see how the "same logical mixup" could lead to both
p = malloc(sizeof gadget);
and
p = malloc(*p);
The first comes from forgetting the type of *p, an error that's not
particular unusual in code that has a large number of user-defined
types. The second comes from forgetting to type sizeof, a very different
type of error, and one I would not describe as a "logical mixup".
I'm sorry that I totally failed to make my point, due to silly syntax
errors.
James Kuyper
2017-03-25 01:10:22 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
I just realized that your example is a syntax error unless gadget is the
name of a variable. I had thought it was meant to be a typedef name. Had
you intended to write
p = malloc(sizeof(gadget));
Yes, that was my intention. Sorry for the mistake.
...
Post by David Brown
Another silly mistake from me. I must have made that post in a rush.
I assumed as much - I just had no idea what your example would have
looked like if written as intended.
Post by David Brown
p = malloc(sizeof *p);
is not /wrong/. I just wonder if it would make it harder to spot a
logical flaw in the program if the user has got his types mixed up.
I don't see how. A type mix-up is guaranteed to not be a problem with
this version of the code; once the user realizes that, it frees him to
pay more attention to lines of code that might actually be affected by
such a mixup.
David Brown
2017-03-25 01:37:51 UTC
Reply
Permalink
Raw Message
Post by James Kuyper
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
I just realized that your example is a syntax error unless gadget is the
name of a variable. I had thought it was meant to be a typedef name. Had
you intended to write
p = malloc(sizeof(gadget));
Yes, that was my intention. Sorry for the mistake.
...
Post by David Brown
Another silly mistake from me. I must have made that post in a rush.
I assumed as much - I just had no idea what your example would have
looked like if written as intended.
Post by David Brown
p = malloc(sizeof *p);
is not /wrong/. I just wonder if it would make it harder to spot a
logical flaw in the program if the user has got his types mixed up.
I don't see how. A type mix-up is guaranteed to not be a problem with
this version of the code; once the user realizes that, it frees him to
pay more attention to lines of code that might actually be affected by
such a mixup.
My point was that if the user thinks he is dealing with a
pointer-to-gadget when he in fact has a pointer-to-widget, then his code
is going to be wrong even if the allocation happens to be safe. Longer
distance between the parts of the code that mention the types can make
it harder to see what has gone wrong.

Of course, there are many other methods to make such mistakes easier to
avoid or find - better choice of variable names, shorter functions,
declaring variables only when you are going to initialise them, using
factory or creator functions, etc.

Having the name of the type in the malloc line might be a help too.
Whether that is a useful enough point to make it worth putting the type
name there is another matter - but it /might/ make code clearer to some
people.
James Kuyper
2017-03-25 01:45:59 UTC
Reply
Permalink
Raw Message
...
Post by David Brown
Post by James Kuyper
Post by David Brown
p = malloc(sizeof *p);
is not /wrong/. I just wonder if it would make it harder to spot a
logical flaw in the program if the user has got his types mixed up.
I don't see how. A type mix-up is guaranteed to not be a problem with
this version of the code; once the user realizes that, it frees him to
pay more attention to lines of code that might actually be affected by
such a mixup.
My point was that if the user thinks he is dealing with a
pointer-to-gadget when he in fact has a pointer-to-widget, then his code
is going to be wrong even if the allocation happens to be safe.
The parts of his code which are actually wrong is where he should be
expecting to get a warning message, and if he doesn't, he should
concentrate on writing them in such a way as to get that warning.
Writing this line of code in a sub-optimal way just to get a warning
that should properly be attached to some other line of code is not the
best way to go about it.
Manfred
2017-03-25 14:58:48 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by James Kuyper
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
I just realized that your example is a syntax error unless gadget is the
name of a variable. I had thought it was meant to be a typedef name. Had
you intended to write
p = malloc(sizeof(gadget));
Yes, that was my intention. Sorry for the mistake.
...
Post by David Brown
Another silly mistake from me. I must have made that post in a rush.
I assumed as much - I just had no idea what your example would have
looked like if written as intended.
Post by David Brown
p = malloc(sizeof *p);
is not /wrong/. I just wonder if it would make it harder to spot a
logical flaw in the program if the user has got his types mixed up.
I don't see how. A type mix-up is guaranteed to not be a problem with
this version of the code; once the user realizes that, it frees him to
pay more attention to lines of code that might actually be affected by
such a mixup.
<snip>
Post by David Brown
Having the name of the type in the malloc line might be a help too.
Whether that is a useful enough point to make it worth putting the type
name there is another matter - but it /might/ make code clearer to some
people.
I, FWIW, tend to prefer p = malloc(sizeof *p); for the reasons that both
Richard and James pointed out. However, you have some point in this last
part - I regard type safety as highly valued.
That said, looking closely at this point, I think that p =
malloc(sizeof(widget)); still does not best answer your point. To me it
looks that, aiming to make the type this explicit, the programmer should
go for

widget* p = malloc(sizeof *p);

i.e. avoid malloc as assignment, and use it for initialization instead
(which has by definition built-in type declaration)
If this requires changing a few extra lines of code, IMHO this is work
well spent.
In a situation where it would be redundant to re-declare the type, as in

void create_widget(widget** pp)
{
*pp = malloc(sizeof **pp);
...
}

then the declaration of pp (as here being an argument) should be close
enough to avoid confusion about its type. Furthermore, a valid
alternative is also

void create_widget(widget** pp)
{
widget *p = malloc(sizeof *p);
...

*pp = p;
}
Keith Thompson
2017-03-24 19:44:27 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by James Kuyper
...
Post by David Brown
While malloc only cares about the size of the allocation, I think the
type /is/ important here. If you have "widget *p; p = malloc(sizeof
gadget);", then the code almost certainly has a big bug even if "gadget"
and "widget" are the same size. The allocation will be fine, but you've
got a logical bug somewhere.
Yes, but "sizeof *p" avoids that problem perfectly, without making any
mention of the type.
No, it makes this bug even harder to find.
If the user writes "p = malloc(sizeof gadget)" when p is declared as
"widget *", then you can see that they are mixing up gadgets and
widgets. If the user, with the same logical mixup, writes "p =
malloc(*p)", then it is harder to spot the logical flaw.
I don't understand. Did you mean to omit the "sizeof" keyword in your
example?

If you write

p = malloc(*p);

then that's almost certainly a bug, and one that the compiler probably
won't catch if *p has a numeric type. But if you write

p = malloc(sizeof *p);

then you *can't* mix up gadgets and widgets. If p is a pointer to
gadget, it allocates space for one gadget; if p is a pointer to widget,
it allocates space for one widget.
Post by David Brown
The point is, there is a bug somewhere. If you are lucky, the compiler
can help find it. Less lucky, you can see it from the code.
I am not saying that writing "malloc(sizeof gadget)" is necessarily a
good idea overall - but it may have some merits.
If gadget is a type, this has to be `malloc(sizeof (gadget))`.

I presume that you're making a valid point, but I'm unable to determine
what it is. Can you clarify?

[...]

There is one valid point I can think of here. It's reasonable to argue
that this:

p = (gadget*)malloc(sizeof (gadget));

is safer than:

p = malloc(sizeof (gadget));

If p is of type gadget*, both are correct. If p is of type widget*,
then both are incorrect, but only the first will be caught by the
compiler. So *if* you use `sizeof (type_name)` rather than `sizeof
*pointer_object`, and *if* you manage to keep the two references to the
type name consistent, then a cast can help the compiler detect errors
that it would not detect without the cast.

But I still maintain that

p = malloc(sizeof *p);

is better than either. It doesn't includes a reminder that p points to
a widget, but no such reminder is needed at that point. If you need to
remember what p points to, look at its declaration, or at the code that
uses it.

Similarly, I can write

n ++;

without having to specify which numeric type n has. I could remind
myself that it's, say, an unsigned int by writing:

n = (unsigned int)(n + 1);

but that's hardly an improvement. There's a simple way to write it that
doesn't require specifying the type. (This case is admittedly different
because, unlike pointers, numeric types can be implicitly converted, so
the above probably won't be diagnosed even if n is a signed char.)
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
s***@casperkitty.com
2017-03-24 19:07:35 UTC
Reply
Permalink
Raw Message
Post by David Brown
I do have a copy of MISRA, and I'd guess very roughly that 60% of the
rules are quite good ideas, 20% are not particularly helpful, and 20%
are downright bad. Many of the good ones are pretty obvious (like "do
not rely on undefined behaviour"). Many of the ones I consider bad had
are motivated by poor tools - a developer (especially for the kinds of
software where MISRA is appropriate) should get better tools rather than
write less clear code. And some are a matter of style and opinion.
A fair portion of the rules stem from the fact that the C Standard makes
no effort to require that all implementations be suitable for purposes
served by MISRA, and thus affords compilers a degree of flexibility which
is counter-productive toward such aims.

MISRA, for example, takes the view that all computations should involve
precisely-sized operators, and programmers should perform all type
conversions manually, but C has no such concept. Given:

uint16_t x,y;

code needing the mod-65536 product of x and y can't simply multiply them,
nor even multiply them and cast the result to uint16_t. To be assured of
reliable operation one would need to do something like (uint16_t)(1u*x*y).
MISRA doesn't like the type "unsigned", but using the unsigned constant
1u is probably the least-bad way of preventing 32-bit compilers from
malfunctioning in cases where x*y would exceed 2147483647 without
encouraging 16-bit compilers to performing a silly 32x32 multiply.

Also, while it's easy to take the attitude that lower-quality tools should
be replaced, replacing a tool may often require requalifying all code that
was processed using it. If the code in question has been working 100%
reliably for decades, there can be significant advantages to continuing to
use the existing code and tooling.
David Brown
2017-03-24 23:54:41 UTC
Reply
Permalink
Raw Message
Post by s***@casperkitty.com
Post by David Brown
I do have a copy of MISRA, and I'd guess very roughly that 60% of the
rules are quite good ideas, 20% are not particularly helpful, and 20%
are downright bad. Many of the good ones are pretty obvious (like "do
not rely on undefined behaviour"). Many of the ones I consider bad had
are motivated by poor tools - a developer (especially for the kinds of
software where MISRA is appropriate) should get better tools rather than
write less clear code. And some are a matter of style and opinion.
Also, while it's easy to take the attitude that lower-quality tools should
be replaced, replacing a tool may often require requalifying all code that
was processed using it. If the code in question has been working 100%
reliably for decades, there can be significant advantages to continuing to
use the existing code and tooling.
Replacing your /compiler/ can certainly be a big tasks, requiring
requalifying code. And for some targets, decent compilers may not be
available.

But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x) ... " to satisfy MISRA's reasoning that it catches typos like
"if (x = 3) ...". If your compiler cannot warn you that "if (x = 3)
..." is suspicious code, run your code through a different compiler for
checking - or use "pc-lint" or similar programs.
Richard Bos
2017-03-25 10:16:06 UTC
Reply
Permalink
Raw Message
Post by David Brown
But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x) ... " to satisfy MISRA's reasoning that it catches typos like
"if (x = 3) ...". If your compiler cannot warn you that "if (x = 3)
..." is suspicious code, run your code through a different compiler for
checking - or use "pc-lint" or similar programs.
It's even worse than that. Many (maybe most, I can't be gluteused to
check) comparisons are between objects, not one object and one constant.
People who write if (3 == x) think they're safe, and then they write
if (y = x) by accident. People who write if (x == 3) check out of habit,
and therefore will not write if (x = y) unless they intend to.

Richard
Richard Heathfield
2017-03-25 12:32:45 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by David Brown
But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x)
(David: how is that "Yoda style"? The applicable grammar here is C, not
English. Do you think comparison imposes a grammatical ordering on
comparands?)
Post by David Brown
... " to satisfy MISRA's reasoning that it catches typos like
Post by David Brown
"if (x = 3) ...". If your compiler cannot warn you that "if (x = 3)
..." is suspicious code, run your code through a different compiler for
checking - or use "pc-lint" or similar programs.
It's even worse than that. Many (maybe most, I can't be gluteused to
check) comparisons are between objects, not one object and one constant.
People who write if (3 == x) think they're safe,
Not all of them.
Post by David Brown
and then they write if (y = x) by accident.
And then they search their C source for = to ensure that every
assignment is supposed to be an assignment and every comparison is meant
to be a comparison, just like those who write x == 3. In fact, the only
difference between the x == 3-er and the 3 == x-er (all else being
equal) is that the 3 == x-er gains the benefit of the compiler's help if
he happened to miss something (and indeed many compilers will in any
case flag up x = 3 anyway).
Post by David Brown
People who write if (x == 3) check out of habit,
Except, of course, for those who don't check. Likewise, people who write
if(3 == x) check out of habit, except for those who don't.
Post by David Brown
and therefore will not write if (x = y) unless they intend to.
Well, they might not.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
David Brown
2017-03-25 14:17:36 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by David Brown
Post by David Brown
But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x)
(David: how is that "Yoda style"? The applicable grammar here is C, not
English. Do you think comparison imposes a grammatical ordering on
comparands?)
"Yoda style" means writing things in an odd order for some imagined
safety or efficiency benefit. It is normal (for English speakers,
anyway) to say "if x is 3 then ...". Thus the clearest and most natural
C equivalent is "if (x == 3) ...".

If like Yoda you want to talk, and to say "if 3 is x ..." you prefer,
then "if (3 == x)" you write.
Post by Richard Heathfield
Post by David Brown
... " to satisfy MISRA's reasoning that it catches typos like
Post by David Brown
"if (x = 3) ...". If your compiler cannot warn you that "if (x = 3)
..." is suspicious code, run your code through a different compiler for
checking - or use "pc-lint" or similar programs.
It's even worse than that. Many (maybe most, I can't be gluteused to
check) comparisons are between objects, not one object and one constant.
People who write if (3 == x) think they're safe,
Not all of them.
Post by David Brown
and then they write if (y = x) by accident.
And then they search their C source for = to ensure that every
assignment is supposed to be an assignment and every comparison is meant
to be a comparison, just like those who write x == 3. In fact, the only
difference between the x == 3-er and the 3 == x-er (all else being
equal) is that the 3 == x-er gains the benefit of the compiler's help if
he happened to miss something (and indeed many compilers will in any
case flag up x = 3 anyway).
The difference is that "if (x == 3)" is more natural to write (and read)
- and thus better than "if (3 == x)".

Writing "if (3 == x)" has a benefit /only/ if you have poor tools (or
don't know how to use them correctly) which are unable to warn you if
you write "if (x = 3)".
Post by Richard Heathfield
Post by David Brown
People who write if (x == 3) check out of habit,
Except, of course, for those who don't check. Likewise, people who write
if(3 == x) check out of habit, except for those who don't.
Post by David Brown
and therefore will not write if (x = y) unless they intend to.
Well, they might not.
Richard Heathfield
2017-03-25 16:32:50 UTC
Reply
Permalink
Raw Message
Post by David Brown
Post by Richard Heathfield
Post by David Brown
But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x)
(David: how is that "Yoda style"? The applicable grammar here is C, not
English. Do you think comparison imposes a grammatical ordering on
comparands?)
"Yoda style" means writing things in an odd order for some imagined
safety or efficiency benefit. It is normal (for English speakers,
anyway) to say "if x is 3 then ...". Thus the clearest and most natural
C equivalent is "if (x == 3) ...".
If like Yoda you want to talk, and to say "if 3 is x ..." you prefer,
then "if (3 == x)" you write.
You are still confusing English and C.

x == y doesn't mean "x is y". Nor does it mean "y is x". It means that
its two operands evaluate to the same value.
Post by David Brown
Writing "if (3 == x)" has a benefit /only/ if you have poor tools (or
don't know how to use them correctly) which are unable to warn you if
you write "if (x = 3)".
Writing if(3 == x) does indeed have at least that benefit. Writing if(x
== 3) has no commensurate benefit.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Keith Thompson
2017-03-25 17:15:39 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by David Brown
Post by Richard Heathfield
Post by David Brown
But there are plenty of other tools available for static error checking.
There is /no/ excuse for writing Yoda-style conditionals such as "if
(3 == x)
(David: how is that "Yoda style"? The applicable grammar here is C, not
English. Do you think comparison imposes a grammatical ordering on
comparands?)
"Yoda style" means writing things in an odd order for some imagined
safety or efficiency benefit. It is normal (for English speakers,
anyway) to say "if x is 3 then ...". Thus the clearest and most natural
C equivalent is "if (x == 3) ...".
If like Yoda you want to talk, and to say "if 3 is x ..." you prefer,
then "if (3 == x)" you write.
You are still confusing English and C.
x == y doesn't mean "x is y". Nor does it mean "y is x". It means that
its two operands evaluate to the same value.
`x == y` means "x is equal to y" (I think that's a good English
rendition of the C semantics).
Post by Richard Heathfield
Post by David Brown
Writing "if (3 == x)" has a benefit /only/ if you have poor tools (or
don't know how to use them correctly) which are unable to warn you if
you write "if (x = 3)".
Writing if(3 == x) does indeed have at least that benefit. Writing if(x
== 3) has no commensurate benefit.
Of course `x == 3` and `3 == x` mean exactly the same thing in C. But
for me, and for a *lot* of people, `x == 3` is the natural way to write
it, and `3 == x` is almost painfully awkward. If you don't find it
awkward, that's great, but a lot of us do.

In English, the meaning of "Found someone you have I would say, hmm?" is
perfectly clear, but we don't talk that way unless we're specifically
imitating Yoda, because it's awkward. I find `3 == x` equally awkward.
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
James R. Kuyper
2017-03-25 18:07:43 UTC
Reply
Permalink
Raw Message
On 03/25/2017 01:15 PM, Keith Thompson wrote:
...
Post by Keith Thompson
In English, the meaning of "Found someone you have I would say, hmm?" is
perfectly clear, but we don't talk that way unless we're specifically
imitating Yoda, because it's awkward. I find `3 == x` equally awkward.
There's a key difference - Yoda's use of "English" (obviously he's
speaking some other language, represented as English in the movies) is
not merely awkward, it often violates English grammar rules. The words
themselves contain enough information for us to reconstruct what a
grammatically correct version of the same statement would look like -
but that's probably only because the writers would not have bothered
presenting us with any of the cases where Yoda's preferred word ordering
is so different from that of "English" as to render his meaning
incomprehensible. Possibly Yoda himself has learned to avoid those cases.

3==x does not violate C's grammar rules, so I can't see the two examples
as "equally awkward".
Richard Heathfield
2017-03-25 18:23:02 UTC
Reply
Permalink
Raw Message
On 25/03/17 17:15, Keith Thompson wrote:

<snip>
Post by Keith Thompson
Of course `x == 3` and `3 == x` mean exactly the same thing in C.
Yes.
Post by Keith Thompson
But
for me, and for a *lot* of people, `x == 3` is the natural way to write
it, and `3 == x` is almost painfully awkward. If you don't find it
awkward, that's great, but a lot of us do.
And likewise, if you do find it awkward, that's unfortunate, but a lot
of us don't.
Post by Keith Thompson
In English, the meaning of "Found someone you have I would say, hmm?" is
perfectly clear, but we don't talk that way unless we're specifically
imitating Yoda, because it's awkward. I find `3 == x` equally awkward.
That's still a very English way of looking at it.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Ike Naar
2017-03-25 18:48:22 UTC
Reply
Permalink
Raw Message
Post by Keith Thompson
Of course `x == 3` and `3 == x` mean exactly the same thing in C. But
for me, and for a *lot* of people, `x == 3` is the natural way to write
it, and `3 == x` is almost painfully awkward. If you don't find it
awkward, that's great, but a lot of us do.
In English, the meaning of "Found someone you have I would say, hmm?" is
perfectly clear, but we don't talk that way unless we're specifically
imitating Yoda, because it's awkward. I find `3 == x` equally awkward.
I would say that a C expression resembles a mathematical expression
more than that it resembles English prose.

In mathematics, it is not unusual for a constant to appear on the
left side of an equality. See, for instance, Stephen Wolfram's web
page that lists several expansions for the constant 'pi',

http://functions.wolfram.com/Constants/Pi/06/01/01

Here's an example from that page (paraphrased because some
mathematical symbols are hard to reproduce in plain text):

pi = 4 * (Sum k : k >= 0 : (-1)^k / (2*k+1))

Is this really more awkward than

4 * (Sum k : k >= 0 : (-1)^k / (2*k+1)) = pi

?
~
Anton Shepelev
2017-03-25 20:22:58 UTC
Reply
Permalink
Raw Message
In mathematics, it is not unusual for a constant
to appear on the left side of an equality. See,
for instance, Stephen Wolfram's web page that
lists several expansions for the constant 'pi',
http://functions.wolfram.com/Constants/Pi/06/01/01
Here's an example from that page (paraphrased be-
cause some mathematical symbols are hard to repro-
pi = 4 * (Sum k : k >= 0 : (-1)^k / (2*k+1))
Is this really more awkward than
4 * (Sum k : k >= 0 : (-1)^k / (2*k+1)) = pi
?
The natural arrangement has nothing to do with which
side is a constant and which is not, but rather with
which expression is the definition and which the
thing defined.

The first equality answers the question "How may Pi
be defined?" whereas the second "What is the sum of
this infinite series?"

Likewise, in C, "x == 3" is a question about x,
whether it equals three, or a statment about x, that
it equals three. Expressing it as "3 == x" would be
ridiculous because it would seem a question or stat-
ment about the number three, whereas it is the vari-
able x that is the actual subject of our query.
--
() ascii ribbon campaign - against html e-mail
/\ http://preview.tinyurl.com/qcy6mjc [archived]
Ike Naar
2017-03-25 19:35:25 UTC
Reply
Permalink
Raw Message
Post by David Brown
"Yoda style" means writing things in an odd order for some imagined
safety or efficiency benefit. It is normal (for English speakers,
anyway) to say "if x is 3 then ...". Thus the clearest and most natural
C equivalent is "if (x == 3) ...".
English is not my native language, but at school I was taught that
"x == 3" is not read as "x is three", but as "x equals three", and
to my ears that sounds just as acceptable as "three equals x".

Does the Yoda problem extend to, say, the plus operator as well?
Is "epsilon + 1" ("epsilon plus one") okay, while "1 + epsilon"
("one plus epsilon") sounds awkward?
Jerry Stuckle
2017-03-24 19:12:23 UTC
Reply
Permalink
Raw Message
Post by j***@verizon.net
...
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
That's not news. Most standards have defects (ISO C definitely included). In
fact, I'd be hard pressed to believe that any standard was perfect. For example,
every requirement from MISRA that's been mentioned in this group has been, at
best, poorly motivated, and at worst, positively wrong headed. I don't own a
copy, so I can't be sure, but I expect it contains a great many reasonable
requirements - but it's the others that are disproportionately likely to be
discussed here.
That's why I'm only interested in the technical reasons you will never be able
to give for this preference, not the argument from authority which is the only
thing you ever bother to give.
I read the explanation CERT gave for their requirement, and that was based
entirely upon the unstated but incorrect assumption that the type, rather than
the size, was important.
Well, if you think this standard is incorrect, you are quite able to
recommend CERT change it. But you need a good *technical* reason to do
so. And that I have yet to see from anyone in this group.

And yes, CERT is a widely recognized authority on the subject. Much
more so than anyone in this newsgroup.

CERT's reasoning is quite valid - the type is important, and size is
directly dependent on type.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Ian Collins
2017-03-24 19:41:53 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
"John Bode suggested the following compliant solution". Apparently not.
--
Ian
Joe Pfeiffer
2017-03-24 22:27:16 UTC
Reply
Permalink
Raw Message
Post by Ian Collins
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
"John Bode suggested the following compliant solution". Apparently not.
Watching him carefully ignore the fact that the authority he is
appealing to in fact accepts the p = malloc(sizeof *p) version is
possibly the funniest part of this exchange.
James Kuyper
2017-03-25 01:36:40 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
Post by Ian Collins
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
"John Bode suggested the following compliant solution". Apparently not.
Watching him carefully ignore the fact that the authority he is
appealing to in fact accepts the p = malloc(sizeof *p) version is
possibly the funniest part of this exchange.
To be fair, while Robert Seacord has described this as a "compliant
solution", I don't see any way to interpret the wording of the actual
recommendation as allowing this construct to be considered compliant,
and Stuckle's immediate response when first informed of that comment was
Post by Joe Pfeiffer
Which is *noncomplient*. Reasonable or not.
Secondly, the current recommendation is not Seacord's. He is listed as
the person who created this recommendation, but the recommendation he
created was quite different. Seacord himself wrote: "Originally the
recommendation was to not cast, which upset a lot of people ... The
current recommendation was written by Dan Saks, ...".
Jerry Stuckle
2017-03-25 00:44:40 UTC
Reply
Permalink
Raw Message
Post by Ian Collins
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
"John Bode suggested the following compliant solution". Apparently not.
Yes, he suggested a compliant solution. That does not mean it was
accepted as a compliant solution. But you're too dense to understand
the difference.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Ian Collins
2017-03-25 01:46:47 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Ian Collins
Post by Jerry Stuckle
And CERT, a highly respected organization, has standards which disagree
with you.
"John Bode suggested the following compliant solution". Apparently not.
Yes, he suggested a compliant solution. That does not mean it was
accepted as a compliant solution. But you're too dense to understand
the difference.
Jolly good.
--
Ian
s***@casperkitty.com
2017-03-24 18:24:14 UTC
Reply
Permalink
Raw Message
Post by j***@verizon.net
p = (int_least64_t*)malloc(count * sizeof(int_least32_t));
The compiler would provide no help at catching this error, assuming that int_least64_t* is the correct type for p. Since the things that don't match are on the same line, it's pretty easy to catch such errors, but despite that fact I have seen precisely this type of mistake committed a couple of time.
The better approach has an analogous failure mode - if the name of a variable changes, a careless coder could leave you with
q = malloc(count * sizeof *p);
However, changes in the name of a variable generally (in my experience) mean that the old variable name is no longer in use, so the compiler will typically be able to point out such errors to you.
Both failure modes include gross localized failures on the part of the
programmer. The former is perhaps more obviously wrong, especially if the
thing being written is a complicated lvalue expression and the sizeof
expression uses a simpler one which *should* have the same type.

The only common situations where I would expect the type of "p" could be
changed without requiring lots of rework would be if all uses of that type
use a common typedef. Given

typedef uint16_t thingIndex;
...
thingIndex *p;

and

p = (thingIndex*)malloc(sizeof (thingIndex*));

Changing the first declaration to:

typedef uint32_t thingIndex;
...
thingIndex *p;

would not require any change to the malloc() code. Changing declarations
to:

typedef uint16_t thingIndex;
...
uint32_t *p;

would break the malloc() code, but it would likely also break a lot of
other code as well--some of it silently. I would not consider the fact
that the latter change wouldn't break the malloc() as an advantage, since
having wrong code generate compiler squawks is a good thing.
Scott Lurndal
2017-03-24 13:52:19 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.
I'm really not sure why you continue to devolve to argumentum ad verecundiam.
Richard Heathfield
2017-03-24 13:55:44 UTC
Reply
Permalink
Raw Message
<snip>
Post by Scott Lurndal
Post by Jerry Stuckle
Post by Richard Heathfield
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.
I'm really not sure why you continue to devolve to argumentum ad verecundiam.
Are you sure you're not sure why? I'm pretty sure /I/'m sure why.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 14:31:30 UTC
Reply
Permalink
Raw Message
Post by Scott Lurndal
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.
I'm really not sure why you continue to devolve to argumentum ad verecundiam.
Maybe because the real experts on the language know more than the idiots
here who spend all day arguing the standard and scope of variables.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 14:39:48 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Scott Lurndal
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.
I'm really not sure why you continue to devolve to argumentum ad verecundiam.
Maybe because the real experts on the language know more than the idiots
here who spend all day arguing the standard and scope of variables.
You are answering a question about why you appeal to inappropriate
authority by appealing to inappropriate authority. Has it ever occurred
to you to base your answers on your own understanding of C rather than
that of other people? Or is that not an option for you?
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 16:54:12 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Scott Lurndal
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
Yes, I know. But it's still a bad idea.
So you know more than CERT. ROFLMAO!
I don't claim to know more than CERT about everything. But I suspect I
know more than some of them do about some aspects of C.
As for your opinion, it is of no consequence because it is based on
authority rather than on facts. Even very knowledgeable people can make
mistakes that less knowledgeable people might recognise as
mistakes, and
so the idea that "expert says this but you say that, therefore you are
wrong" is misguided. When an expert says something stupid like "cast
malloc", it's as well to recognise that he's saying something stupid
despite being an expert.
If you want to argue the case on its merits, feel free, but arguing the
case purely from an appeal to authority has no value. If you are able to
defend your position on *technical* grounds, go ahead.
But these are not just one person's view. They are the consensus of a
large number of very knowledgeable people. And a large number of other
C experts have known over the years agree with CERT.
I'm really not sure why you continue to devolve to argumentum ad verecundiam.
Maybe because the real experts on the language know more than the idiots
here who spend all day arguing the standard and scope of variables.
You are answering a question about why you appeal to inappropriate
authority by appealing to inappropriate authority. Has it ever occurred
to you to base your answers on your own understanding of C rather than
that of other people? Or is that not an option for you?
The authority in this case is CERT - a world-wide recognized group of
experts, who have produced those standards.

As opposed to Richard, an idiot who can't do any better than argue
standards and quote dictionary definitions.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Jorgen Grahn
2017-03-23 21:56:28 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
But they seem to nullify it all in the end with:

"MEM02-C-EX1: Do not immediately cast the results of malloc() for code
that will be compiled using a C90-conforming compiler [...]|"

Am I misinterpreting, or do they really say "ignore this advice unless
your compiler is more than thirty years old"?

I'm not a big fan of coding standards. (But I like that these guys
allow and participate in open discussions about their
recommendations.)

/Jorgen
--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
Keith Thompson
2017-03-23 22:18:00 UTC
Reply
Permalink
Raw Message
[...]
Post by Jorgen Grahn
Post by Jerry Stuckle
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
"MEM02-C-EX1: Do not immediately cast the results of malloc() for code
that will be compiled using a C90-conforming compiler [...]|"
Am I misinterpreting, or do they really say "ignore this advice unless
your compiler is more than thirty years old"?
Yes, I think that's what they mean.

C90 lets you call a function with no visible declaration, but it
assumes that the function returns int. If you omit the required
`#include <stdlib.h>`, then a call without a cast:

int *p = malloc(sizeof *p);

will cause a diagnostic (assigning an int value to an int* object),
but a call with a cast:

int *p = (int*)malloc(sizeof *p);

likely will not. But the cast will convert the nonexistent int result
to int*, causing undefined behavior.

In C99 and later, you don't have that particular problem, since omitting
`#include <stdlib.h>` should trigger a diagnostic with or without the
cast.

The exception makes sense *if* you think the cast is a good idea.

But if you omit the cast in all cases, you'll have more robust code that
compiles without complaint in C90, C99, or C11. (Pre-ANSI compilers
might complain, but presumably they're rarely an issue.)

[...]
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Joe Pfeiffer
2017-03-23 22:39:09 UTC
Reply
Permalink
Raw Message
Post by Jorgen Grahn
Post by Jerry Stuckle
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
"MEM02-C-EX1: Do not immediately cast the results of malloc() for code
that will be compiled using a C90-conforming compiler [...]|"
Am I misinterpreting, or do they really say "ignore this advice unless
your compiler is more than thirty years old"?
I believe they are saying "you should ignore this advice if your
compiler *is* more than thirty years old".
Post by Jorgen Grahn
I'm not a big fan of coding standards. (But I like that these guys
allow and participate in open discussions about their
recommendations.)
Jerry Stuckle
2017-03-24 00:42:47 UTC
Reply
Permalink
Raw Message
Post by Jorgen Grahn
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
"MEM02-C-EX1: Do not immediately cast the results of malloc() for code
that will be compiled using a C90-conforming compiler [...]|"
Am I misinterpreting, or do they really say "ignore this advice unless
your compiler is more than thirty years old"?
I'm not a big fan of coding standards. (But I like that these guys
allow and participate in open discussions about their
recommendations.)
/Jorgen
Or maybe they're saying "ignore this advice unless you compiler is less
than 27 years old".
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Joe Pfeiffer
2017-03-23 22:33:57 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.

Note that their noncompliant example is

p = malloc(sizeof(gadget)); /* Imminent problem */

The version I had above would be (in CERT's example)

p = malloc(sizeof *p);

This mitigates their concern about changing the type of p without
updating the malloc() to match.
Jerry Stuckle
2017-03-24 00:45:58 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
Note that their noncompliant example is
p = malloc(sizeof(gadget)); /* Imminent problem */
The version I had above would be (in CERT's example)
p = malloc(sizeof *p);
This mitigates their concern about changing the type of p without
updating the malloc() to match.
Or, maybe demonstrating YOU have it wrong. But CERT would NEVER be more
aware than you! ROFLMAO!
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 10:01:14 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.

Some of us did that.

They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.

And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 13:27:22 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.
Some of us did that.
They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.
And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
Yes, they accepted some of the ideas. But they didn't accept the
idiotic ones.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 13:53:48 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.
Some of us did that.
They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.
And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
Yes, they accepted some of the ideas. But they didn't accept the
idiotic ones.
Technical arguments only, please. Calling an idea idiotic doesn't make
it idiotic. If you want to convince people, you have to show *why* it's
idiotic. "It's idiotic because CERT disagrees" is not convincing.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 14:32:39 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.
Some of us did that.
They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.
And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
Yes, they accepted some of the ideas. But they didn't accept the
idiotic ones.
Technical arguments only, please. Calling an idea idiotic doesn't make
it idiotic. If you want to convince people, you have to show *why* it's
idiotic. "It's idiotic because CERT disagrees" is not convincing.
You have yet to provide ANY technical argument as to why they are wrong.
And I'm not calling the idea idiotic - I'm calling the people here who
spend all day arguing the standard and the scope of variables idiots.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
Richard Heathfield
2017-03-24 14:42:50 UTC
Reply
Permalink
Raw Message
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.
Some of us did that.
They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.
And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
Yes, they accepted some of the ideas. But they didn't accept the
idiotic ones.
Technical arguments only, please. Calling an idea idiotic doesn't make
it idiotic. If you want to convince people, you have to show *why* it's
idiotic. "It's idiotic because CERT disagrees" is not convincing.
You have yet to provide ANY technical argument as to why they are wrong.
Not so, but I'll reiterate. They're wrong to attempt to mandate a style
that introduces unnecessary code without benefit. They're wrong to
attempt to mandate a style that can hide a C90 bug. And they're wrong to
attempt to mandate a style that is less robust than the obvious
alternative in the face of changing requirements.
Post by Jerry Stuckle
And I'm not calling the idea idiotic - I'm calling the people here who
spend all day arguing the standard and the scope of variables idiots.
Still no technical arguments, then. Okay.
--
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within
Jerry Stuckle
2017-03-24 16:56:26 UTC
Reply
Permalink
Raw Message
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Jerry Stuckle
Post by Richard Heathfield
Post by Joe Pfeiffer
Post by Jerry Stuckle
Post by Richard Heathfield
<snip>
Post by Joe Pfeiffer
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that it is simpler, the cast is unnecessary, and changing
the type of q at some point in the future automatically still works
without remembering to change the argument to sizeof.
It's also much easier to read, and easier to see that it's correct.
Post by Joe Pfeiffer
Others regard the
longer version as being a bit of self-documentation.
And, like all documentation, it runs the danger of becoming outdated.
It is also recommended by the SEI CERT C Coding Standard at
https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard
https://www.securecoding.cert.org/confluence/display/c/MEM02-C.+Immediately+cast+the+result+of+a+memory+allocation+function+call+into+a+pointer+to+the+allocated+type
Demonstrating that CERT gets it wrong sometimes.
CERT came here a few years ago to ask people here to review their
proposed coding standard.
Some of us did that.
They (for the most part) ignored the result, as a result of which the
CERT Standard contains serious flaws.
And then they wrote some of us up in their acknowledgements page, as if
we somehow endorse the result.
Yes, they accepted some of the ideas. But they didn't accept the
idiotic ones.
Technical arguments only, please. Calling an idea idiotic doesn't make
it idiotic. If you want to convince people, you have to show *why* it's
idiotic. "It's idiotic because CERT disagrees" is not convincing.
You have yet to provide ANY technical argument as to why they are wrong.
Not so, but I'll reiterate. They're wrong to attempt to mandate a style
that introduces unnecessary code without benefit. They're wrong to
attempt to mandate a style that can hide a C90 bug. And they're wrong to
attempt to mandate a style that is less robust than the obvious
alternative in the face of changing requirements.
Post by Jerry Stuckle
And I'm not calling the idea idiotic - I'm calling the people here who
spend all day arguing the standard and the scope of variables idiots.
Still no technical arguments, then. Okay.
You haven't produced any. But widely recognized experts disagree with
the great Richard, who can't do more than argue standards and dictionary
definitions.
--
==================
Remove the "x" from my email address
Jerry Stuckle
***@attglobal.net
==================
s***@casperkitty.com
2017-03-24 15:45:58 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
The version I had above would be (in CERT's example)
p = malloc(sizeof *p);
This mitigates their concern about changing the type of p without
updating the malloc() to match.
Code which uses "malloc" generally performs other actions to initialize the
newly-created storage. There are some cases where it might be possible to
change "*p" to a different type without having to adjust other code, but in
general code which uses "malloc" will be dependent upon the type being created
in ways other than its size. Given code like:

p = (fnord*)malloc(sizeof (fnord));
p->wizzle = 3;
p->wozzle = 6;

the explicit mention of the type within the "malloc" expression makes clear
that the code is expecting to create a "fnord" object. If the code had
instead been:

p = malloc(sizeof *p);
p->wizzle = 3;
p->wozzle = 6;

and the type of "*p" were changed from "fnord" to a "fnoord" which is similar
but has an extra field, then the code would create a "fnoord" whose extra
field was uninitialized. In some cases that would be acceptable, but I see
no reason to blindly expect that it would be. The former pattern would result
in the compiler calling the programmer's attention to the code. If the
programmer determined that the code would be just as appropriate for type
"fnoord" as it had been for "fnord", the programmer could change the types
in the malloc and be done with it, but the diagnostic would give a wise
programmer a chance to inspect the surrounding code first to ensure that was
the *only* change that was necessary. The latter program, by contrast, would
not call attention to itself even if attention were required.
Jorgen Grahn
2017-03-23 21:14:20 UTC
Reply
Permalink
Raw Message
...
Post by Joe Pfeiffer
Post by a***@gmail.com
Also I came across this code on another website
http://quiz.geeksforgeeks.org/queue-set-2-linked-list-implementation/
struct Queue *q = createQueue();
...
Post by Joe Pfeiffer
(Two things I noticed that aren't actually relevant to your question,
Many people would prefer to see
struct Queue *q = (struct Queue*)malloc(sizeof(struct Queue));
replaced with
struct Queue *q = malloc(sizeof *q);
on the grounds that [...]
I agree.

Mentioning another thing: I'd rather not see that malloc() at all. The
nodes in the queue pretty much need to be malloc()ed, but the struct
Queue doesn't have to. I might want to keep it in a struct, an array,
or just as a local variable.

So I'd prefer createQueue() to look like:

struct Queue foo;
initQueue(&foo);
// or
struct Queue bar = createQueue();
// or
struct Queue baz = EMPTY_QUEUE;

I don't want to get the OP stuck in a pattern where everything is
malloc()ed and everything becomes a pointer. Memory management is
tricky enough as it is ...

/Jorgen
--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
Joe Pfeiffer
2017-03-23 17:52:19 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
a***@gmail.com
2017-03-23 18:03:50 UTC
Reply
Permalink
Raw Message
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.

http://codepad.org/4IiDVzpT

Thanks for your replies.
a***@gmail.com
2017-03-23 18:14:03 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Joe Pfeiffer
2017-03-23 18:41:36 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Since q is external to any functions, it actually is initialized to NULL
before the program begins. So in this case, yes, it has been
initialized. If it were a local variable, its value would be
indeterminate.

Down at the bottom of the page, it says:

Output:
1 Segmentation fault

So, as expected, this example contains an error that causes the program
to crash.
Keith Thompson
2017-03-23 18:48:04 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Either the "extra code" was necessary for correctness, or the code in
the book is incorrect.

At file scope, you have:

struct queue *q;//Here q is not initialized

q is not *explicitly* initialized. It's implicitly initialized to NULL.
Any file-scope and/or static object without an initializer is implicitly
initialized to zero; for a pointer object, that's a null pointer.

int main()
{
int val, option;
create_queue(q);//just called create queue. At this moment nothing such as struct node is created

At this point, q still has its initial null value, so the call is
equivalent to create_queue(NULL). Since function arguments are always
passed by value, this cannot change the value of q.

void create_queue(struct queue *q)
{
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
}

This dereferences a null pointer, causing undefined behavior.
You were lucky enough to get a segmentation fault, a clear indication
that there's a problem.
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
a***@gmail.com
2017-03-23 18:59:46 UTC
Reply
Permalink
Raw Message
Post by Keith Thompson
Post by a***@gmail.com
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Either the "extra code" was necessary for correctness, or the code in
the book is incorrect.
struct queue *q;//Here q is not initialized
q is not *explicitly* initialized. It's implicitly initialized to NULL.
Any file-scope and/or static object without an initializer is implicitly
initialized to zero; for a pointer object, that's a null pointer.
int main()
{
int val, option;
create_queue(q);//just called create queue. At this moment nothing such as struct node is created
At this point, q still has its initial null value, so the call is
equivalent to create_queue(NULL). Since function arguments are always
passed by value, this cannot change the value of q.
void create_queue(struct queue *q)
{
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
}
This dereferences a null pointer, causing undefined behavior.
You were lucky enough to get a segmentation fault, a clear indication
that there's a problem.
--
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
I can understand now, that the given example is wrong. The segmentation fault is because of missing malloc in creat_queue() function.
I added that malloc and the code runs ok until creat_queue(). Another segmentation fault occurs when insert function is called.

here

struct queue *insert(struct queue *q,int val)
{

struct node *ptr;

ptr = (struct node*)malloc(sizeof(struct node));
ptr->data = val;
if(q->front == NULL)
{
q->front = ptr; //something is wrong here as I keep getting seg fault error in first insertion.
q->rear = ptr;
q->front->next = q->rear->next = NULL;
}
else
{
q->rear->next = ptr;
q->rear = ptr;
q->rear->next = NULL;
}

return q;
}
Keith Thompson
2017-03-23 19:35:38 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
Post by Keith Thompson
Post by a***@gmail.com
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Either the "extra code" was necessary for correctness, or the code in
the book is incorrect.
struct queue *q;//Here q is not initialized
q is not *explicitly* initialized. It's implicitly initialized to NULL.
Any file-scope and/or static object without an initializer is implicitly
initialized to zero; for a pointer object, that's a null pointer.
int main()
{
int val, option;
create_queue(q);//just called create queue. At this moment nothing such as struct node is created
At this point, q still has its initial null value, so the call is
equivalent to create_queue(NULL). Since function arguments are always
passed by value, this cannot change the value of q.
void create_queue(struct queue *q)
{
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
}
This dereferences a null pointer, causing undefined behavior.
You were lucky enough to get a segmentation fault, a clear indication
that there's a problem.
I can understand now, that the given example is wrong. The
segmentation fault is because of missing malloc in creat_queue()
function.
No, that's not the only problem. For that to work, create_queue() would
have to return a `struct queue *` value. (Or it could take a `struct
queue**` argument, but returning the pointer is probably cleaner and
easier to understand.)

Something like this:

struct queue *new_queue(void) {
struct queue *q = malloc(sizeof *q);
if (q != NULL) {
q->rear = NULL;
q->front = NULL;
}
return q;
}

And of course you'll have to assign the returned value to your pointer
object in main or wherever you call it).

Note that I've changed the name of the function to reflect what it
returns rather than what it does.

[...]
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
a***@gmail.com
2017-03-23 19:48:45 UTC
Reply
Permalink
Raw Message
Post by Keith Thompson
Post by a***@gmail.com
Post by Keith Thompson
Post by a***@gmail.com
Post by a***@gmail.com
Post by Joe Pfeiffer
Post by a***@gmail.com
I see example codes where q is never initialised to NULL and a
function is there that says 'createq'. All it do in the function is
just make rear and front to point to NULL. At that moment nothing is
allocated to anyone.
Either you're misunderstanding the examples, or they are wrong. Do you
have a URL?
I am reading a book. I have formatted and removed extra code for better readability.
http://codepad.org/4IiDVzpT
Thanks for your replies.
Book name is data structures using C - Reema Thareja
Either the "extra code" was necessary for correctness, or the code in
the book is incorrect.
struct queue *q;//Here q is not initialized
q is not *explicitly* initialized. It's implicitly initialized to NULL.
Any file-scope and/or static object without an initializer is implicitly
initialized to zero; for a pointer object, that's a null pointer.
int main()
{
int val, option;
create_queue(q);//just called create queue. At this moment nothing such as struct node is created
At this point, q still has its initial null value, so the call is
equivalent to create_queue(NULL). Since function arguments are always
passed by value, this cannot change the value of q.
void create_queue(struct queue *q)
{
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
}
This dereferences a null pointer, causing undefined behavior.
You were lucky enough to get a segmentation fault, a clear indication
that there's a problem.
I can understand now, that the given example is wrong. The
segmentation fault is because of missing malloc in creat_queue()
function.
No, that's not the only problem. For that to work, create_queue() would
have to return a `struct queue *` value. (Or it could take a `struct
queue**` argument, but returning the pointer is probably cleaner and
easier to understand.)
struct queue *new_queue(void) {
struct queue *q = malloc(sizeof *q);
if (q != NULL) {
q->rear = NULL;
q->front = NULL;
}
return q;
}
And of course you'll have to assign the returned value to your pointer
object in main or wherever you call it).
Note that I've changed the name of the function to reflect what it
returns rather than what it does.
[...]
--
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
I just came to senses.
The malloc I did in create_queue() is a local memory allocation. q will be again uninitialized after the function returns, hence I was getting segmentation faults. I did correct way.

http://codepad.org/sqUT3bx4


Thanks for the super help.
Keith Thompson
2017-03-23 20:24:25 UTC
Reply
Permalink
Raw Message
***@gmail.com writes:
[...]
Post by a***@gmail.com
I just came to senses.
The malloc I did in create_queue() is a local memory allocation. q
will be again uninitialized after the function returns, hence I was
getting segmentation faults. I did correct way.
Almost. The memory allocated by malloc() isn't local, it's on the
"heap", and its lifetime continues until it's deallocated by free() or
equivalent, or until the program completes.

What's local is your only pointer to it. When that pointer object
vanishes, you've lost your only reference to the allocated memory.
It's a memory leak.

And q isn't "again uninitialized". You actually have two distinct
objects named "q". One is a global (file-scope) variable. The other
is the parameter to the create_queue() function, which is initialized
with a *copy* of the value of the argument, the global "q". (Parameters
are like local variables, except that they're initialized to the value
of the corresponding argument.)
Post by a***@gmail.com
http://codepad.org/sqUT3bx4
Here's your updated create_queue() function:

struct queue *create_queue(struct queue *q)
{
q = malloc(sizeof(struct queue));
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
return q;
}

It takes an argument of type "struct queue*", but it doesn't do anything
with that value; it immediately clobbers it with the value returned by
malloc(). This isn't exactly wrong, but it's useless. You can drop the
parameter and change it to a local variable:

struct queue *create_queue(void)
{
struct queue q = malloc(sizeof(struct queue));
q -> rear = NULL;//how this happened without any error??
q -> front = NULL;
return q;
}

Then the call in your main function becomes:

q = create_queue();

Your insert() function always returns the value of its first parameter,
which it never changes. It might as well be a void function, not
returning a value. (Unless it *should* modify the value of q; I haven't
studied the logic closely enough to be sure.)

Consistent indentation would make your code much easier to read.
--
Keith Thompson (The_Other_Keith) kst-***@mib.org <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Scott Lurndal
2017-03-23 20:46:52 UTC
Reply
Permalink
Raw Message
Post by a***@gmail.com
ptr = (struct node*)malloc(sizeof(struct node));
ptr->data = val;
And if the malloc fails?
Barry Schwarz
2017-03-24 02:02:55 UTC
Reply
Permalink
Raw Message
On Thu, 23 Mar 2017 09:56:59 -0700 (PDT), ***@gmail.com
wrote:

Your subject is self contradictory. You should never attempt to
execute a program that does not compile cleanly. It should also link
cleanly.
--
Remove del for email
Loading...