From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 92656A0565; Mon, 23 Mar 2020 13:10:08 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 362E51C044; Mon, 23 Mar 2020 13:10:07 +0100 (CET) Received: from mail-io1-f67.google.com (mail-io1-f67.google.com [209.85.166.67]) by dpdk.org (Postfix) with ESMTP id E89671C036 for ; Mon, 23 Mar 2020 13:10:05 +0100 (CET) Received: by mail-io1-f67.google.com with SMTP id h131so13872164iof.1 for ; Mon, 23 Mar 2020 05:10:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=SZR0mJO4H5QjaByXeIdZgDWDvG/fsmjA8ZPCZMALLJo=; b=jBg0EMIevAKy6RgzZxf6iOp0Oy3vQV541mjBcvmQ9Bb1wsTDtjo+Yq+mjYma7NBJxb dZWzBxiyEKZ6tGWzalE/njN02fwKTeUcs5POYWwR4U3WPgL2e9/12QambBZYlBm19nG7 LVZ0fqAJ+Q5plXduELsyKZmSvpERR4HGHj1NlwU/iQhJ5/KGn9U1eYQ6Mb8WbeB1Pv+r k3ldbuCEXqAXAhsho6nHcZpy1UBoZLdwkkSxVwRaakypZONpsGrzRBj8LfQWr/oqQnlV xli0SvksyhUGivEI4G6cK6OXmQl3Zc+nHyroojqKGCpI2wmC+NRGWSU1hvHrdpB39kKR fAlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=SZR0mJO4H5QjaByXeIdZgDWDvG/fsmjA8ZPCZMALLJo=; b=nTkStrAt12bzuI5kyPADDY7IU8Ro1mAkWexNUXlVq8Tk91qix2d2T3IcgTOBb78C5R CGyDsT4Rb13SmOADuSm/Ux3fVsaDk8nnr1g+nESmdi1xdNVEvCk0xW4Elwx90+GCOKw8 00lISxLQbobWroGh+yFByH75tzY25k9GtdR0/JbZL9s2uCpstQo+gC7JAgdll2PzQh+v aJC/Yrz2NYX/Da6XL1kyqd6Z7AmXlt81F8oJCjp6/O3iheqKxURGnIXOf9vB131/z6HZ OS08X4KCr2Sd8aOkAqTBVzgMvELaSe72cT3BZ8ALdwQwpaDNZm/pvsFVh+Q7rX+0q42Y fuLA== X-Gm-Message-State: ANhLgQ0BpXhpuGzlu+NLbFw71m4/ihM0Dtcn17eJcxSezKNg6bAkBwCI 1VKQnGMJFkcJ7nWuzVNG91go51qn49Vzreya6AM= X-Google-Smtp-Source: ADFU+vsPf5aU98ztDt0NxGGifvger1nX9k54hEjvqT5MTlyVh50PYscBBdKDr0gZihtVOvaNyNGWIaNQu4X2CXgcpv0= X-Received: by 2002:a6b:e316:: with SMTP id u22mr7550625ioc.1.1584965405028; Mon, 23 Mar 2020 05:10:05 -0700 (PDT) MIME-Version: 1.0 References: <20200318190241.3150971-1-jerinj@marvell.com> <20200318190241.3150971-6-jerinj@marvell.com> <438b28d3-68dc-e55c-2ae2-2617451fa276@ericsson.com> In-Reply-To: <438b28d3-68dc-e55c-2ae2-2617451fa276@ericsson.com> From: Jerin Jacob Date: Mon, 23 Mar 2020 17:39:49 +0530 Message-ID: To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Cc: "jerinj@marvell.com" , Sunil Kumar Kori , Bruce Richardson , "dev@dpdk.org" , "thomas@monjalon.net" , "david.marchand@redhat.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v1 05/32] eal/trace: add internal trace init and fini interface X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Mar 19, 2020 at 4:05 PM Mattias R=C3=B6nnblom wrote: > > On 2020-03-18 20:02, jerinj@marvell.com wrote: > > From: Jerin Jacob > > +static bool > > +trace_entry_compare(const char *name) > > +{ > > + struct trace_point_head *tp_list =3D trace_list_head_get(); > > + struct trace_point *tp; > > + int count =3D 0; > > + > > + STAILQ_FOREACH(tp, tp_list, next) { > > + if (strncmp(tp->name, name, TRACE_POINT_NAME_SIZE) =3D=3D= 0) > > + count++; > > + if (count > 1) { > > + trace_err("found duplicate entry %s", name); > > + rte_errno =3D EEXIST; > > + return 1; > > Maybe an assertion would be better here, especially since the caller > doesn't seem to care about the fact the list is inconsistent. assertion/panic are getting removed from the libraries. This error propagates up to rte_eal_init() failure. So, the current scheme is OK. > > Change 1 -> true. Will fix it in v2. > > > + } > > + } > > + return 0; > return false; Will fix it in v2. > > +} > > + > > +static int > > +trace_dir_default_path_get(char *dir_path) > > +{ > > + struct trace *trace =3D trace_obj_get(); > > + uint32_t size =3D sizeof(trace->dir); > > + struct passwd *pwd; > > + char *home_dir; > > + > > + /* First check for shell environment variable */ > > + home_dir =3D getenv("HOME"); > In case of the application being started with 'sudo', $HOME will not be > "/root", but rather the original user. This sounds like a bug to me. When the application started with sudo, the trace will be written by the "root" user. Probably it is not good to have some file whose ownership as root in /home/user/dpdk-trace/ where the user can not delete it. A similar scheme followed in LTTng for trace directory. I am open to a better scheme if any. > > + if (home_dir =3D=3D NULL) { > > + /* Fallback to password file entry */ > > + pwd =3D getpwuid(getuid()); > > + if (pwd =3D=3D NULL) > > + return -EINVAL; > > + > > + home_dir =3D pwd->pw_dir; > > + } > > + > > + /* Append dpdk-traces to directory */ > > + if (snprintf(dir_path, size, "%s/dpdk-traces/", home_dir) < 0) > > + return -ENAMETOOLONG; > > + > > + return 0; > > +} > > + > > +int > > +trace_mkdir(void) > > +{ > > + struct trace *trace =3D trace_obj_get(); > > + char session[TRACE_DIR_STR_LEN]; > > + char *dir_path; > > + int rc; > > + > > + if (!trace->dir_offset) { > > + dir_path =3D (char *)calloc(1, sizeof(trace->dir)); > calloc() returns a void pointer, so no need to cast it. Will fix it in v2. > > + if (dir_path =3D=3D NULL) { > > + trace_err("fail to allocate memory\n"); > > + return -ENOMEM; > > + } > > + > > + rc =3D trace_dir_default_path_get(dir_path); > > + if (rc < 0) { > > + trace_err("fail to get default path\n"); > > + free(dir_path); > > + return rc; > > + } > > + > > + } > > + > > + /* Create the path if it t exist, no "mkdir -p" available here */ > > + rc =3D mkdir(trace->dir, 0700); > > + if (rc < 0 && errno !=3D EEXIST) { > > + trace_err("mkdir %s failed [%s]", trace->dir, strerror(er= rno)); > > + rte_errno =3D errno; > > + return -rte_errno; > > + } > > + > > + rc =3D trace_session_name_generate(session); > > + if (rc < 0) > > + return rc; > > + > > + rc =3D mkdir(trace->dir, 0700); > > + if (rc < 0) { > > + trace_err("mkdir %s failed [%s]", trace->dir, strerror(er= rno)); > > + rte_errno =3D errno; > > + return -rte_errno; > > + } > > + > > + RTE_LOG(INFO, EAL, "Trace dir: %s\n", trace->dir); > > + return 0; > > Is dir_path leaked? I can't find a free(), except in one of the error cas= es. Not is in "struct trace" as memory. struct trace { char dir[PATH_MAX]; .. }