GPS serial memory leak?

[tsteven4]

GPS_Serial_On allocates memory for the device header on both windows

gpsbabel/jeeps/gpsserial.cc

Lines 94 to 100 in ad23c57

	int32_t GPS_Serial_On(const char* port, gpsdevh** dh)
	{
	DCB tio;
	COMMTIMEOUTS timeout;
	HANDLE comport;
	const char* xname = fix_win_serial_name(port);
	win_serial_data* wsd = (win_serial_data*) xcalloc(sizeof(win_serial_data), 1);

and posix

gpsbabel/jeeps/gpsserial.cc

Lines 573 to 575 in ad23c57

	int32_t GPS_Serial_On(const char* port, gpsdevh** dh)
	{
	auto* psd = (posix_serial_data*) xcalloc(sizeof(posix_serial_data), 1);

GPS_Serial_Off frees the memory on windows

gpsbabel/jeeps/gpsserial.cc

Lines 164 to 169 in ad23c57

	int32_t GPS_Serial_Off(gpsdevh* dh)
	{
	win_serial_data* wsd = (win_serial_data*)dh;
	CloseHandle(wsd->comport);
	wsd->comport = INVALID_HANDLE_VALUE;
	xfree(wsd);

but not posix.

gpsbabel/jeeps/gpsserial.cc

Lines 599 to 608 in ad23c57

	int32_t GPS_Serial_Off(gpsdevh* dh)
	{
	if (!GPS_Serial_Close(dh)) {
	GPS_Error("Error Closing port");
	gps_errno = HARDWARE_ERROR;
	return 0;
	}
	return 1;
	}

[robertlipe]

Back at home where I can review on something bigger than a phone... It's probable that the obvious one-line fix is the correct one. It's *possible* that it'll corrupt memory in some weird way. The serial/USB paths have not been subject to the valgrind/memtest rigor that has tormented the rest of the code. In addition to being a pain to run via automation, the reality is that the runs are necessarily shorter (I think even now that the biggest garmin won't take more than 1,000 wpts through the USB protocol. Maybe it's 5,000.) and are less likely to be tied up in a long-running daemon process than the rest of the code. Code touches the device, moves data through, exits. A leak just isn't a big deal. I think I brought on the devh stuff as part of the USB work in '04 or so. It was definitely retrofitted in with a big hammer, even then. If you want to go ahead and just add the "obvious" xfree(dh); go for it. If there's a problem, assign this to me and I'll find and attach a serial and/or USB device and track down any exposed issue. I'd respect you no less for just backing <https://media2.giphy.com/media/COYGe9rZvfiaQ/giphy.gif?cid=ecf05e47afm9dzbthsmbzrymgmtckb4sovwcwiu0zuh55wil&ep=v1_gifs_search&rid=giphy.gif&ct=g> away. RJL

…

On Wed, Nov 15, 2023 at 6:07 PM tsteven4 ***@***.***> wrote: GPS_Serial_On allocates memory for the device header on both windows https://github.com/GPSBabel/gpsbabel/blob/ad23c5767a149e71381503810677ba75df95c471/jeeps/gpsserial.cc#L94-L100 and posix https://github.com/GPSBabel/gpsbabel/blob/ad23c5767a149e71381503810677ba75df95c471/jeeps/gpsserial.cc#L573-L575 GPS_Serial_Off frees the memory on windows https://github.com/GPSBabel/gpsbabel/blob/ad23c5767a149e71381503810677ba75df95c471/jeeps/gpsserial.cc#L164-L169 but not posix. https://github.com/GPSBabel/gpsbabel/blob/ad23c5767a149e71381503810677ba75df95c471/jeeps/gpsserial.cc#L599-L608 — Reply to this email directly, view it on GitHub <#1221>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD35EXCKSWZPPHU4VJK3YEVKN7AVCNFSM6AAAAAA7NJOFCGVHI2DSMVQWIX3LMV43ASLTON2WKOZRHE4TKNZYHAZDAMI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[tsteven4]

Thanks for your reply. The leak is confirmed.