Re: About Information Hiding Design Rules

On 07/01/2008 08:58 PM, Eric Sosman wrote:
matt wrote:
On 07/01/2008 07:16 PM, Eric Sosman wrote:
However, this problem mostly goes away if you adopt the more
usual arrangement. Since myadt_create() is the source of all
myadt_t instances, making sure that myadt_create() initializes
the new instances properly solves the problem. Your client has
no way to create a myadt_t instance on its own, so your client
has no opportunity to forget to initialize one.

You are right. It's unlikely the client to forget to initialize an instance of myadt_t...


What do you mean with "properly"? Adopting this solution should suffice

myadt_t *
myadt_create(void)
{
myadt_t * ptr;
ptr = malloc (sizeof(myadt_t));

Consider `ptr = malloc(sizeof *ptr);' as a safer alternative,
"safer" because there's less chance of an accidental mismatch.
Many's the time I've seen errors like

msghead_t *p = malloc(sizeof(msghead_t));
msgbody_t *q = malloc(sizeof(msghead_t));

... which the suggested idiom almost always avoids.

if (ptr == NULL) {
printf("Memory couldn't be allocated\n");

Consider writing error messages to stderr instead of to
stdout. Still better, consider writing no message at all, but
just returning the NULL and letting the client decide what to
do about the matter; the client has broader knowledge of the
context than you do.

return 0;
}
ptr->x = 10;
ptr->y = 10;
return ptr;
}

... isn't it?

Yes, this is what I meant: myadt_create() either returns
NULL or returns a pointer to a myadt_t instance that has been
initialized and is ready for use. There is no possibility that
the program can create a myadt_t instance in any other way,
hence no possibility of an uninitialized instance.

but, hey, errare humanum est! Please consider this main() program:

#include <stdio.h>
#include "module.h"
int
main()
{
myadt_t * ADT;
int status;

/*
* a lot of work here... (1)
*/

myadt_get_status(ADT, &status);
printf("status: %d\n", status);
return 1;
}

Here, being a very absent-minded man :), because of (1) I forgot to initialize an instance of myadt_t. As result I obtain:

matt@isaac:~/work/c/information-hiding$ ./exe
status: 953015467

So, I wonder what do you think about the following solution:
============
module.c
============
....
struct myadt_t {
int x;
int y;
int uninitialised_data;
};
....

myadt_t *
myadt_create(void)
{
...
ptr->uninitialised_data = 0;
return ptr;
}
....

int
myadt_get_status(myadt_t * ptr, int * status)
{
if (ptr->uninitialised_data) {
printf("Working with uninitialized data struct\n");
return 0;
}
...
return 1;
}

In practice, IMHO, it's highly unlikely that the pointer ADT (in main()) contains the address of a memory location in which the integer corresponding to "int uninitialized_data" has all its 32 bits set to zero. Therefore, the test

if (ptr->uninitialised_data) ...

always returns true, except after the function myadt_create() has been called. Indeed, running the program I get:

matt@isaac:~/work/c/information-hiding$ ./exe
Working with uninitialized data struct

Would this solution be so awful?
.

Relevant Pages

  • Re: Error when assigning char* to structure member
    ... 14 int main ... You can only initialize global variables with values that are known at ... this case is not an lvalue and the compiler is ... To highlight the difference try to imagine how "STRING" and ptr are ...
    (comp.lang.c)
  • [NT] Dark Age of Camelot Man-In-The-Middle
    ... use of RSA public key cryptography and an RC4 based symmetric algorithm. ... Seeing the imminent release of code for cracking the game client (which ... At the beginning of each TCP session, the server sends a 1536 bit RSA ... void bytes_out(unsigned char *data, int len) ...
    (Securiteam)
  • up-imapproxy DoS vulnerabilities
    ... up-imapproxy is an IMAP proxy which keeps connections open after client has ... and reuses it when client connects back. ... There are various bugs in up-imapproxy which can crash it. ... unsigned int ReadBytesProcessed; /* bytes already processed in read buf */ ...
    (Bugtraq)
  • [PATCH 4/7] rename locking functions - convert init_MUTEX* users, part 2
    ... struct apple_thermal_info *info; ... int __init attach_aci_rds ... /* initialize structs / fill in defaults */ ...
    (Linux-Kernel)
  • [UNIX] Heap Overflow in SNMPNetStat (Exploit Code)
    ... static int session_id; ... xorl %eax, %eax ... snmp_waiting_getnext2 (int sd, u_char *buf) ... u_char *ptr = buf; ...
    (Securiteam)