Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Re: Request for review

Reply
Thread Tools

Re: Request for review

 
 
James Antill
Guest
Posts: n/a
 
      07-24-2003
On Thu, 24 Jul 2003 10:41:25 -0700, Bertrand Mollinier Toublet wrote:

> Hi,
>
> I wrote an implementation of a dynamic array (i.e. you trade the
> language syntax support for an array unbounded on one end).
> I posted the code (including API specification, in the header file)
> online at http://www.bmt.dnsalias.org/employment, in the project
> section, at the bottom of the page.
> Please review and comment /ad lib/.


Well, it looks like it works . Although you should check for overflow
of the length in create/resize

Some style comments, you have...

typedef struct da_dyn_array_s da_dyn_array_t;
typedef struct da_iterator da_iterator_t;

1. I'd capitalize all typedef's due to them being in the same namespace as
struct members.

2. *_t is in the POSIX namespace, so you might want to not usre that
suffix.

3. The _s suffix on the struct should be on both, or my preference would
be not at all.

The implementation of db_create is...

da_dyn_array_t *new_da;

new_da = malloc(sizeof *new_da);
if (NULL != new_da)
{
new_da->base = malloc(nmemb * size);
if (NULL == new_da->base)
{
free(new_da);
new_da = NULL;
}
else
{
new_da->nmemb = nmemb;
new_da->size = size;
new_da->index = 0;
}
}
return new_da;

....this looks much nicer as (esp. with the extra checks)...

da_dyn_array_t *new_da = NULL;
size_t bytes = 0;

new_da = malloc(sizeof *new_da);
if (NULL == new_da)
goto malloc_da_fail;

if (!nmemb) /* don't allow this, so we can check the args */
nmemb = 1; /* also gets rid of corner case in resize */

bytes = nmemb * size;
if ((0 == bytes) || ((bytes / size) != nmemb))
goto check_args_fail;

new_da->base = malloc(nmemb * size);
if (NULL == new_da->base)
goto malloc_base_fail;

new_da->nmemb = nmemb;
new_da->size = size;
new_da->index = 0;

return new_da;

malloc_base_fail:
check_args_fail:
free(new_da);

malloc_da_fail:
return (NULL);

....but as I said, they aren't required ... just comments.

--
James Antill -- http://www.velocityreviews.com/forums/(E-Mail Removed)
Need an efficent and powerful string library for C?
http://www.and.org/vstr/

 
Reply With Quote
 
 
 
 
Bertrand Mollinier Toublet
Guest
Posts: n/a
 
      07-24-2003
James Antill wrote:
> On Thu, 24 Jul 2003 10:41:25 -0700, Bertrand Mollinier Toublet wrote:
>
>
>>Hi,
>>
>>I wrote an implementation of a dynamic array (i.e. you trade the
>>language syntax support for an array unbounded on one end).
>>I posted the code (including API specification, in the header file)
>>online at http://www.bmt.dnsalias.org/employment, in the project
>>section, at the bottom of the page.
>>Please review and comment /ad lib/.

>
>
> Well, it looks like it works . Although you should check for overflow
> of the length in create/resize
>
> bytes = nmemb * size;
> if ((0 == bytes) || ((bytes / size) != nmemb))
> goto check_args_fail;
>

James,

thanks for the review. Checking for overflow definitely is a good idea.
Although, I found the following in the bugs list of your library:

.. size overflows, just don't do it (tm) ... (Ie. having a Vstr string of
length (SIZE_MAX - 1) and adding 2 bytes to it will just break.

What's up with that ?


--
Bertrand Mollinier Toublet
"Uno no se muere cuando debe, sino cuando puede"
-- Cor. Aureliano Buendia

 
Reply With Quote
 
 
 
 
James Antill
Guest
Posts: n/a
 
      07-26-2003
On Fri, 25 Jul 2003 09:11:46 +0000, Dan Pop wrote:

> In <(E-Mail Removed)> "James Antill" <(E-Mail Removed)> writes:
>
>>1. I'd capitalize all typedef's due to them being in the same namespace as
>>struct members.

>
> Where did you get this idea from?


Hmm, I thought gcc had warned me for having a memeber the same as a
typedef I was getting from the zlib library at one point ... but all
recent versions don't seem to. The only thing I can find in c99 is 6.7.7.6
but gcc doesn't support unnamed memebers even now and the override in the
global variable isn't illegal, although gcc does warn about that.

Looks like that's not true.

--
James Antill -- (E-Mail Removed)
Need an efficent and powerful string library for C?
http://www.and.org/vstr/

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Review: Razer eXactMat Review Silverstrand Reviews & How-To's 0 06-20-2005 03:04 AM
Review: NZXT Guardian Case Review Silverstrand Reviews & How-To's 0 06-20-2005 02:56 AM
Review: Silverstone Gloria SST-TJ04 Case Review Silverstrand Reviews & How-To's 0 06-20-2005 02:53 AM
Review: Battalion-101~ S Notebook Review Silverstrand Reviews & How-To's 0 06-20-2005 02:52 AM
Re: Accessing Request.InputStream / Request.BinaryRead *as the request is occuring*: How??? Brian Birtle ASP .Net 2 10-16-2003 02:11 PM



Advertisments