From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Nicol=C3=A1s_Pernas_Maradei?= Subject: Re: =?utf-8?q?=5BPATCH=5D_Fix_librte=5Fpmd=5Fpcap_driver_d?= =?utf-8?q?ouble_stop_error?= Date: Sat, 04 Oct 2014 19:14:21 +0100 Message-ID: <7ab24cf8da0f11aa07b6770883de356a@statler.emutex.com> References: <1410380225-13751-1-git-send-email-nico@emutex.com> <20140929142406.GC26483@hmsreliant.think-freely.org> Reply-To: nico-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Cc: dev-VfR2kkLFssw@public.gmane.org To: Neil Horman Return-path: In-Reply-To: <20140929142406.GC26483-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi, You are correct, the parameters received in the driver are allocated in=20 devargs_list (char *params variable). However, they already get strdup'd=20 in rte_kvargs_parse(). This newly allocated string is part of kvlist and=20 never freed up. The params variable is never used again so it can be=20 freed by someone else using free_devargs_list(). I'd say it's safe=20 enough to set up pointers in the way it's currently done. Nico. On 2014-09-29 15:24, Neil Horman wrote: > On Wed, Sep 10, 2014 at 05:17:05PM -0300, Nicol=C3=A1s Pernas Maradei w= rote: >> From: Nicol=C3=A1s Pernas Maradei >>=20 >> librte_pmd_pcap driver was opening the pcap/interfaces only at init=20 >> time and >> closing them only when the port was being stopped. This behaviour=20 >> would cause >> problems (leading to segfault) if the user closed the port 2 times.=20 >> The first >> time the pcap/interfaces would be normally closed but libpcap would=20 >> throw an >> error causing a segfault if the closed pcaps/interfaces were closed=20 >> again. >> This behaviour is solved by re-openning pcaps/interfaces when the port= =20 >> is >> started (only if these weren't open already for example at init time). >>=20 >> Signed-off-by: Nicol=C3=A1s Pernas Maradei >=20 > This patch assigns pointers to strings that are allocated in the=20 > devargs_list. > Given that there exists an api interface free_devargs_list(), I'm not=20 > sure that > whats being done here is consistently safe. It seems like you should=20 > dup the > strings to make sure you always have the storage allocated, or find=20 > some other > method to store the needed information. >=20 > Neil