From: Paul O. <new...@ki...> - 2008-06-02 06:09:52
|
Hello devels! I'm writing experimental planner plugin for Player. In some place I've found that I don't know what should be done with piece of memory containing request reply (after it is used): msg = this->vectormap_dev->Request(this->InQueue, PLAYER_MSGTYPE_REQ, PLAYER_VECTORMAP_REQ_GET_MAP_INFO, NULL, 0, NULL, true); assert(msg); assert(msg->GetDataSize() > 0); map_info = \ reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); assert(map_info); minx = map_info->extent.x0; miny = map_info->extent.y0; maxx = map_info->extent.x1; maxy = map_info->extent.y1; PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ minx, miny, maxx, maxy); if (map_info->layers) free(map_info->layers); /* not sure about this! */ map_info->layers = NULL; Should it be freed or not? Running this code does not cause any breakdown (doublefree exception or something), so I guess this free() call is right, but I'm not sure. If it is right, I guess many drivers should be checked if they do the same to avoid memleaks. Cheers, Paul |
From: Toby C. <tco...@pl...> - 2008-06-02 19:42:19
|
In general for player structures you should use the relevant player_<type>_free method instead of free. These methods do a deep free of any arrays in the structure. However, the player Message class does this already in its destructor so you should instead delete the message pointer itself. This does not apply to the client libraries where responses must be cleaned up by the caller (but this is generally hidden from end users by the proxy classes). Toby 2008/6/2 Paul Osmialowski <new...@ki...>: > > Hello devels! > > I'm writing experimental planner plugin for Player. In some place I've > found that I don't know what should be done with piece of memory > containing request reply (after it is used): > > msg = this->vectormap_dev->Request(this->InQueue, > PLAYER_MSGTYPE_REQ, > PLAYER_VECTORMAP_REQ_GET_MAP_INFO, > NULL, 0, NULL, true); > assert(msg); > assert(msg->GetDataSize() > 0); > map_info = \ > reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); > assert(map_info); > minx = map_info->extent.x0; > miny = map_info->extent.y0; > maxx = map_info->extent.x1; > maxy = map_info->extent.y1; > PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ > minx, miny, maxx, maxy); > if (map_info->layers) free(map_info->layers); /* not sure about this! */ > map_info->layers = NULL; > > Should it be freed or not? Running this code does not cause any breakdown > (doublefree exception or something), so I guess this free() call is right, > but I'm not sure. If it is right, I guess many drivers should be checked > if they do the same to avoid memleaks. > > Cheers, > Paul > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2008. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > Playerstage-developers mailing list > Pla...@li... > https://lists.sourceforge.net/lists/listinfo/playerstage-developers > -- This email is intended for the addressee only and may contain privileged and/or confidential information |
From: Paul O. <new...@ki...> - 2008-06-02 21:44:39
|
On Tue, 3 Jun 2008, Toby Collett wrote: > In general for player structures you should use the relevant > player_<type>_free method instead of free. These methods do a deep free of > any arrays in the structure. However, the player Message class does this > already in its destructor so you should instead delete the message pointer > itself. This does not apply to the client libraries where responses must be > cleaned up by the caller (but this is generally hidden from end users by the > proxy classes). > > Toby Hi Toby, I didn't paste that, but later I'm doing delete msg; and to my surprise it does not cause doublefree error! How come? Paul > > 2008/6/2 Paul Osmialowski <new...@ki...>: > >> >> Hello devels! >> >> I'm writing experimental planner plugin for Player. In some place I've >> found that I don't know what should be done with piece of memory >> containing request reply (after it is used): >> >> msg = this->vectormap_dev->Request(this->InQueue, >> PLAYER_MSGTYPE_REQ, >> PLAYER_VECTORMAP_REQ_GET_MAP_INFO, >> NULL, 0, NULL, true); >> assert(msg); >> assert(msg->GetDataSize() > 0); >> map_info = \ >> reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); >> assert(map_info); >> minx = map_info->extent.x0; >> miny = map_info->extent.y0; >> maxx = map_info->extent.x1; >> maxy = map_info->extent.y1; >> PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ >> minx, miny, maxx, maxy); >> if (map_info->layers) free(map_info->layers); /* not sure about this! */ >> map_info->layers = NULL; >> >> Should it be freed or not? Running this code does not cause any breakdown >> (doublefree exception or something), so I guess this free() call is right, >> but I'm not sure. If it is right, I guess many drivers should be checked >> if they do the same to avoid memleaks. >> >> Cheers, >> Paul >> >> >> ------------------------------------------------------------------------- >> This SF.net email is sponsored by: Microsoft >> Defy all challenges. Microsoft(R) Visual Studio 2008. >> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ >> _______________________________________________ >> Playerstage-developers mailing list >> Pla...@li... >> https://lists.sourceforge.net/lists/listinfo/playerstage-developers >> > > > > -- > This email is intended for the addressee only and may contain privileged > and/or confidential information > |
From: Toby C. <tco...@pl...> - 2008-06-03 06:53:20
|
Not sure on that one, either player is leaking memory, or the double free checking is not perfect... Toby 2008/6/3 Paul Osmialowski <new...@ki...>: > > > On Tue, 3 Jun 2008, Toby Collett wrote: > > > In general for player structures you should use the relevant > > player_<type>_free method instead of free. These methods do a deep free > of > > any arrays in the structure. However, the player Message class does this > > already in its destructor so you should instead delete the message > pointer > > itself. This does not apply to the client libraries where responses must > be > > cleaned up by the caller (but this is generally hidden from end users by > the > > proxy classes). > > > > Toby > > Hi Toby, > I didn't paste that, but later I'm doing delete msg; and to my surprise it > does not cause doublefree error! How come? > > Paul > > > > > 2008/6/2 Paul Osmialowski <new...@ki...>: > > > >> > >> Hello devels! > >> > >> I'm writing experimental planner plugin for Player. In some place I've > >> found that I don't know what should be done with piece of memory > >> containing request reply (after it is used): > >> > >> msg = this->vectormap_dev->Request(this->InQueue, > >> PLAYER_MSGTYPE_REQ, > >> PLAYER_VECTORMAP_REQ_GET_MAP_INFO, > >> NULL, 0, NULL, true); > >> assert(msg); > >> assert(msg->GetDataSize() > 0); > >> map_info = \ > >> reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); > >> assert(map_info); > >> minx = map_info->extent.x0; > >> miny = map_info->extent.y0; > >> maxx = map_info->extent.x1; > >> maxy = map_info->extent.y1; > >> PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ > >> minx, miny, maxx, maxy); > >> if (map_info->layers) free(map_info->layers); /* not sure about this! > */ > >> map_info->layers = NULL; > >> > >> Should it be freed or not? Running this code does not cause any > breakdown > >> (doublefree exception or something), so I guess this free() call is > right, > >> but I'm not sure. If it is right, I guess many drivers should be checked > >> if they do the same to avoid memleaks. > >> > >> Cheers, > >> Paul > >> > >> > >> > ------------------------------------------------------------------------- > >> This SF.net email is sponsored by: Microsoft > >> Defy all challenges. Microsoft(R) Visual Studio 2008. > >> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > >> _______________________________________________ > >> Playerstage-developers mailing list > >> Pla...@li... > >> https://lists.sourceforge.net/lists/listinfo/playerstage-developers > >> > > > > > > > > -- > > This email is intended for the addressee only and may contain privileged > > and/or confidential information > > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2008. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > Playerstage-developers mailing list > Pla...@li... > https://lists.sourceforge.net/lists/listinfo/playerstage-developers > -- This email is intended for the addressee only and may contain privileged and/or confidential information |
From: bearclaw <bea...@fr...> - 2008-06-03 10:48:34
|
Paul Osmialowski wrote: > On Tue, 3 Jun 2008, Toby Collett wrote: > > >> In general for player structures you should use the relevant >> player_<type>_free method instead of free. These methods do a deep free of >> any arrays in the structure. However, the player Message class does this >> already in its destructor so you should instead delete the message pointer >> itself. This does not apply to the client libraries where responses must be >> cleaned up by the caller (but this is generally hidden from end users by the >> proxy classes). >> >> Toby >> > > Hi Toby, > I didn't paste that, but later I'm doing delete msg; and to my surprise it > does not cause doublefree error! How come? > Because you set it to 0 after free-ing it. Likely the dtor of msg is calling delete and not free, for which 0 is a perfectly legal value. If in doubt use valgrind --leak-check=full. > Paul > > >> 2008/6/2 Paul Osmialowski <new...@ki...>: >> >> >>> Hello devels! >>> >>> I'm writing experimental planner plugin for Player. In some place I've >>> found that I don't know what should be done with piece of memory >>> containing request reply (after it is used): >>> >>> msg = this->vectormap_dev->Request(this->InQueue, >>> PLAYER_MSGTYPE_REQ, >>> PLAYER_VECTORMAP_REQ_GET_MAP_INFO, >>> NULL, 0, NULL, true); >>> assert(msg); >>> assert(msg->GetDataSize() > 0); >>> map_info = \ >>> reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); >>> assert(map_info); >>> minx = map_info->extent.x0; >>> miny = map_info->extent.y0; >>> maxx = map_info->extent.x1; >>> maxy = map_info->extent.y1; >>> PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ >>> minx, miny, maxx, maxy); >>> if (map_info->layers) free(map_info->layers); /* not sure about this! */ >>> map_info->layers = NULL; >>> >>> Should it be freed or not? Running this code does not cause any breakdown >>> (doublefree exception or something), so I guess this free() call is right, >>> but I'm not sure. If it is right, I guess many drivers should be checked >>> if they do the same to avoid memleaks. >>> >>> Cheers, >>> Paul >>> >>> >>> ------------------------------------------------------------------------- >>> This SF.net email is sponsored by: Microsoft >>> Defy all challenges. Microsoft(R) Visual Studio 2008. >>> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ >>> _______________________________________________ >>> Playerstage-developers mailing list >>> Pla...@li... >>> https://lists.sourceforge.net/lists/listinfo/playerstage-developers >>> >>> >> >> -- >> This email is intended for the addressee only and may contain privileged >> and/or confidential information >> >> > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2008. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > Playerstage-developers mailing list > Pla...@li... > https://lists.sourceforge.net/lists/listinfo/playerstage-developers > > |
From: Paul O. <new...@ki...> - 2008-06-03 16:39:17
|
On Tue, 3 Jun 2008, bearclaw wrote: > Paul Osmialowski wrote: >> On Tue, 3 Jun 2008, Toby Collett wrote: >> >> >>> In general for player structures you should use the relevant >>> player_<type>_free method instead of free. These methods do a deep free of >>> any arrays in the structure. However, the player Message class does this >>> already in its destructor so you should instead delete the message pointer >>> itself. This does not apply to the client libraries where responses must be >>> cleaned up by the caller (but this is generally hidden from end users by the >>> proxy classes). >>> >>> Toby >>> >> >> Hi Toby, >> I didn't paste that, but later I'm doing delete msg; and to my surprise it >> does not cause doublefree error! How come? >> > Because you set it to 0 after free-ing it. Likely the dtor of msg is > calling delete and not free, for which 0 is a perfectly legal value. > If in doubt use valgrind --leak-check=full. You're perfectly right bearclaw, not setting layers pointer to NULL causes double free, therefore only delete msg; should be used for freeing request reply. Congratulations for your observation. Cheers, Paul >> >> >>> 2008/6/2 Paul Osmialowski <new...@ki...>: >>> >>> >>>> Hello devels! >>>> >>>> I'm writing experimental planner plugin for Player. In some place I've >>>> found that I don't know what should be done with piece of memory >>>> containing request reply (after it is used): >>>> >>>> msg = this->vectormap_dev->Request(this->InQueue, >>>> PLAYER_MSGTYPE_REQ, >>>> PLAYER_VECTORMAP_REQ_GET_MAP_INFO, >>>> NULL, 0, NULL, true); >>>> assert(msg); >>>> assert(msg->GetDataSize() > 0); >>>> map_info = \ >>>> reinterpret_cast<player_vectormap_info_t *>(msg->GetPayload()); >>>> assert(map_info); >>>> minx = map_info->extent.x0; >>>> miny = map_info->extent.y0; >>>> maxx = map_info->extent.x1; >>>> maxy = map_info->extent.y1; >>>> PLAYER_WARN4("Map extent: %.4f, %.4f - %.4f, %.4f", \ >>>> minx, miny, maxx, maxy); >>>> if (map_info->layers) free(map_info->layers); /* not sure about this! */ >>>> map_info->layers = NULL; >>>> >>>> Should it be freed or not? Running this code does not cause any breakdown >>>> (doublefree exception or something), so I guess this free() call is right, >>>> but I'm not sure. If it is right, I guess many drivers should be checked >>>> if they do the same to avoid memleaks. >>>> >>>> Cheers, >>>> Paul >>>> >>>> >>>> ------------------------------------------------------------------------- >>>> This SF.net email is sponsored by: Microsoft >>>> Defy all challenges. Microsoft(R) Visual Studio 2008. >>>> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ >>>> _______________________________________________ >>>> Playerstage-developers mailing list >>>> Pla...@li... >>>> https://lists.sourceforge.net/lists/listinfo/playerstage-developers >>>> >>>> >>> >>> -- >>> This email is intended for the addressee only and may contain privileged >>> and/or confidential information >>> >>> >> >> ------------------------------------------------------------------------- >> This SF.net email is sponsored by: Microsoft >> Defy all challenges. Microsoft(R) Visual Studio 2008. >> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ >> _______________________________________________ >> Playerstage-developers mailing list >> Pla...@li... >> https://lists.sourceforge.net/lists/listinfo/playerstage-developers >> >> > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2008. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > Playerstage-developers mailing list > Pla...@li... > https://lists.sourceforge.net/lists/listinfo/playerstage-developers > |