Merge lp:~tmaesaka/drizzle/drizzle-fix-drizzleslap into lp:~drizzle-trunk/drizzle/development

Proposed by Toru Maesaka
Status: Merged
Merged at revision: not available
Proposed branch: lp:~tmaesaka/drizzle/drizzle-fix-drizzleslap
Merge into: lp:~drizzle-trunk/drizzle/development
Diff against target: None lines
To merge this branch: bzr merge lp:~tmaesaka/drizzle/drizzle-fix-drizzleslap
Reviewer Review Type Date Requested Status
Brian Aker Needs Resubmitting
Jay Pipes (community) Approve
Stewart Smith (community) Approve
Review via email: mp+6668@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Toru Maesaka (tmaesaka) wrote :

This merge involves three fixes:

(1) There was a block of code that assumed memory allocation
    is always successful so a check is added.

(2) Fix a segmentation fault that is caused due to
    trying to free a invalid pointer in statement_clean().

    Note: This fix is not perfect.

    Ideally, we should move the query_statement list to
    a proper container like std::vector and create a
    nice cleanup function for it (which I'm planning to
    do next). This way, we can get rid of the small
    memory leak that has been in drizzleslap for a while.

(3) Fix a memory leak due to not freeing the string
    obtained inside my_open(). So, use my_close() instead
    of directly closing the descriptor with close(2).

Cheers,
Toru

Revision history for this message
Toru Maesaka (tmaesaka) wrote :

Err, sorry I forgot to mention that all these fixes are for the drizzleslap tool :)

Revision history for this message
Jay Pipes (jaypipes) wrote :

Hi Toru!

I think we've been trying to get rid of calls to my_open() and my_close() in favour of the standard open()/close() methods... so I'd prefer to see the calls to my_open() disappear than going back to my_close() :)

Toru, I can tell you didn't want to go back to my_close()...is there another reason you did that?

-jay

review: Needs Fixing
Revision history for this message
Stewart Smith (stewart) wrote :

Would be great if we removed my_open instead

review: Needs Fixing
Revision history for this message
Toru Maesaka (tmaesaka) wrote :

Jay, Stewart

Hi!

Fair enough I'll get rid of my_open() and resubmit the patch.

> Toru, I can tell you didn't want to go back to my_close()...is there another
> reason you did that?

What I had in mind was to refactor drizzleslap later on and get rid of mysys dependencies then . But I guess I was just being lazy :(

Cheers,
Toru

> Hi Toru!
>
> I think we've been trying to get rid of calls to my_open() and my_close() in
> favour of the standard open()/close() methods... so I'd prefer to see the
> calls to my_open() disappear than going back to my_close() :)
>
> Toru, I can tell you didn't want to go back to my_close()...is there another
> reason you did that?
>
> -jay

1018. By Toru Maesaka

Replace occrrences of my_(open|close) with straight open() and close() system calls

Revision history for this message
Toru Maesaka (tmaesaka) wrote :

Hi!

As suggested in the review, I've replaced all occurrences of my_open() and my_close() in drizzleslap with open(2) and close(2) system calls.

Next stop: Use a real list container for query statements.

Cheers,
Toru

Revision history for this message
Stewart Smith (stewart) :
review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

Approved :) Much better :)

review: Approve
Revision history for this message
Brian Aker (brianaker) wrote :

Hi!

I've asked Toru to take the patch, and apply it to trunk. This tree came off of some tree that must have been abandoned (lots of bad stuff in the merge).

Cheers,
   -Brian

review: Needs Resubmitting
Revision history for this message
Wesley Batista (w-le-y) wrote :

hi

Revision history for this message
api.ng (hektve) wrote :

~hektve/%2Bjunk/BRANCHNAME

Revision history for this message
hunting binocular (huntingbinocular8) wrote :

Best Hunting Binoculars

The <a href="https://thehuntingbinoculars.com/">Best Hunting Binoculars</a> are those that offer excellent optical quality, ruggedness, and durability. Here are some top picks:

Vortex Optics Viper HD Binoculars: These binoculars are known for their excellent optical quality and are ideal for hunting in low light conditions. They come with a lifetime warranty.

Leica Ultravid HD-Plus Binoculars: These binoculars are known for their exceptional clarity and image quality, thanks to their advanced lens coatings. They are also rugged and waterproof.

Swarovski EL Range Binoculars: These binoculars feature excellent optics and an integrated rangefinder, making them an ideal choice for long-range hunting.

Zeiss Victory RF Binoculars: These binoculars also feature an integrated rangefinder, and they are known for their excellent image quality and ruggedness.

Nikon Monarch 5 Binoculars: These binoculars are a more affordable option that still offers excellent optics and durability. They are also lightweight and easy to carry.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'client/drizzleslap.cc'
--- client/drizzleslap.cc 2009-03-26 09:22:55 +0000
+++ client/drizzleslap.cc 2009-05-18 06:43:07 +0000
@@ -1563,7 +1563,7 @@
1563 bytes_read= read(data_file, (unsigned char*) tmp_string,1563 bytes_read= read(data_file, (unsigned char*) tmp_string,
1564 (size_t)sbuf.st_size);1564 (size_t)sbuf.st_size);
1565 tmp_string[sbuf.st_size]= '\0';1565 tmp_string[sbuf.st_size]= '\0';
1566 close(data_file);1566 my_close(data_file, MYF(0));
1567 if (bytes_read != sbuf.st_size)1567 if (bytes_read != sbuf.st_size)
1568 {1568 {
1569 fprintf(stderr, "Problem reading file: read less bytes than requested\n");1569 fprintf(stderr, "Problem reading file: read less bytes than requested\n");
@@ -1620,7 +1620,7 @@
1620 bytes_read= read(data_file, (unsigned char*) tmp_string,1620 bytes_read= read(data_file, (unsigned char*) tmp_string,
1621 (size_t)sbuf.st_size);1621 (size_t)sbuf.st_size);
1622 tmp_string[sbuf.st_size]= '\0';1622 tmp_string[sbuf.st_size]= '\0';
1623 close(data_file);1623 my_close(data_file, MYF(0));
1624 if (bytes_read != sbuf.st_size)1624 if (bytes_read != sbuf.st_size)
1625 {1625 {
1626 fprintf(stderr, "Problem reading file: read less bytes than requested\n");1626 fprintf(stderr, "Problem reading file: read less bytes than requested\n");
@@ -1667,7 +1667,7 @@
1667 bytes_read= read(data_file, (unsigned char*) tmp_string,1667 bytes_read= read(data_file, (unsigned char*) tmp_string,
1668 (size_t)sbuf.st_size);1668 (size_t)sbuf.st_size);
1669 tmp_string[sbuf.st_size]= '\0';1669 tmp_string[sbuf.st_size]= '\0';
1670 close(data_file);1670 my_close(data_file, MYF(0));
1671 if (bytes_read != sbuf.st_size)1671 if (bytes_read != sbuf.st_size)
1672 {1672 {
1673 fprintf(stderr, "Problem reading file: read less bytes than requested\n");1673 fprintf(stderr, "Problem reading file: read less bytes than requested\n");
@@ -1716,7 +1716,7 @@
1716 bytes_read= read(data_file, (unsigned char*) tmp_string,1716 bytes_read= read(data_file, (unsigned char*) tmp_string,
1717 (size_t)(sbuf.st_size));1717 (size_t)(sbuf.st_size));
1718 tmp_string[sbuf.st_size]= '\0';1718 tmp_string[sbuf.st_size]= '\0';
1719 close(data_file);1719 my_close(data_file, MYF(0));
1720 if (bytes_read != sbuf.st_size)1720 if (bytes_read != sbuf.st_size)
1721 {1721 {
1722 fprintf(stderr, "Problem reading file: read less bytes than requested\n");1722 fprintf(stderr, "Problem reading file: read less bytes than requested\n");
@@ -2417,20 +2417,27 @@
2417 uint32_t length= strlen(script);2417 uint32_t length= strlen(script);
2418 uint32_t count= 0; /* We know that there is always one */2418 uint32_t count= 0; /* We know that there is always one */
24192419
2420 for (tmp= *sptr= (statement *)malloc(sizeof(statement));2420 for (tmp= *sptr= (statement *)calloc(1, sizeof(statement));
2421 (retstr= strchr(ptr, delm));2421 (retstr= strchr(ptr, delm));
2422 tmp->next= (statement *)malloc(sizeof(statement)),2422 tmp->next= (statement *)calloc(1, sizeof(statement)),
2423 tmp= tmp->next)2423 tmp= tmp->next)
2424 {2424 {
2425 memset(tmp, 0, sizeof(statement));2425 if (tmp == NULL)
2426 {
2427 fprintf(stderr,"Error allocating memory while parsing delimiter\n");
2428 exit(1);
2429 }
2430
2426 count++;2431 count++;
2427 tmp->length= (size_t)(retstr - ptr);2432 tmp->length= (size_t)(retstr - ptr);
2428 tmp->string= (char *)malloc(tmp->length + 1);2433 tmp->string= (char *)malloc(tmp->length + 1);
2434
2429 if (tmp->string == NULL)2435 if (tmp->string == NULL)
2430 {2436 {
2431 fprintf(stderr,"Error allocating memory while parsing delimiter\n");2437 fprintf(stderr,"Error allocating memory while parsing delimiter\n");
2432 exit(1);2438 exit(1);
2433 }2439 }
2440
2434 memcpy(tmp->string, ptr, tmp->length);2441 memcpy(tmp->string, ptr, tmp->length);
2435 tmp->string[tmp->length]= 0;2442 tmp->string[tmp->length]= 0;
2436 ptr+= retstr - ptr + 1;2443 ptr+= retstr - ptr + 1;