[{"id":4094,"web_url":"https://patchwork.libcamera.org/comment/4094/","msgid":"<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>","date":"2020-03-19T12:52:01","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Kaaira,\n\nOn 19/03/2020 12:42, Kaaira Gupta wrote:\n> The two callers of functions libcameraPath() and isLibcameraInstalled()\n> end up using the same process and finally use the path with\n> libcamera.so. Hence write a function libcameraBuildPath() which\n> combines their functions and returns the root of the build sources\n> when the library has not been installed, but is running,thereby making\n> call sites simpler.\n> \n> When the library is imstalled, libcameraBuildPath() will return an empty\n\n/imstalled/installed/\n\nIf that's the only comment I'll fix while applying.\n\n> string.\n> \n> Make changes in the call sites accordingly.\n> \n> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> ---\n> Changes since v2:\n> \t- rename subject line and add necessary details in commit\n> \t  message.\n> \t- Solve whitespace issues.\n> \t- specify the final 'slash' for directory path in return\n> \t  statement itself.\n> \n> Changes since v1:\n>         - rename function to libcameraBuildPath() instead, and return\n>           the root of libcamera source, and not the source directory\n>           root.\n>         - Return empty string instead of nullptr when ret==0\n> \n>  src/libcamera/include/utils.h |  2 +-\n>  src/libcamera/ipa_manager.cpp |  5 +++--\n>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n>  src/libcamera/utils.cpp       | 16 +++++++++-------\n>  4 files changed, 16 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> index bc96e79..5dea8d2 100644\n> --- a/src/libcamera/include/utils.h\n> +++ b/src/libcamera/include/utils.h\n> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n>  \n>  bool isLibcameraInstalled();\n>  \n> -std::string libcameraPath();\n> +std::string libcameraBuildPath();\n>  \n>  } /* namespace utils */\n>  \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 0bd280c..bcaae35 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n>  \t * path for the IPA from that point. We need to recurse one level of\n>  \t * sub-directories to match the build tree.\n>  \t */\n> -\tif (!utils::isLibcameraInstalled()) {\n> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> +\tstd::string root = utils::libcameraBuildPath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n>  \t\tconstexpr int maxDepth = 1;\n>  \n>  \t\tLOG(IPAManager, Info)\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 2f866cc..5fd88a4 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n>  \t * This requires identifying the path of the libcamera.so, and\n>  \t * referencing a relative path for the proxy workers from that point.\n>  \t */\n> -\tif (!utils::isLibcameraInstalled()) {\n> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> -\t\t\t\t\t  + \"/proxy/worker\";\n> +\tstd::string root = utils::libcameraBuildPath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n>  \n>  \t\tLOG(IPAProxy, Info)\n>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> index 7e118fa..42c5c76 100644\n> --- a/src/libcamera/utils.cpp\n> +++ b/src/libcamera/utils.cpp\n> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n>   *\n>   * \\return A string stating the path\n>   */\n> -std::string libcameraPath()\n> +std::string libcameraBuildPath()\n>  {\n> -\tDl_info info;\n> +\tif (!isLibcameraInstalled()) {\n> +\t\tDl_info info;\n>  \n> -\t/* Look up our own symbol. */\n> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> -\tif (ret == 0)\n> -\t\treturn nullptr;\n> +\t\t/* Look up our own symbol. */\n> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> +\t\tif (ret)\n> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> +\t}\n>  \n> -\treturn info.dli_fname;\n> +\treturn std::string();\n>  }\n>  \n>  } /* namespace utils */\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C179E629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 13:52:05 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1E2CAA53;\n\tThu, 19 Mar 2020 13:52:05 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584622325;\n\tbh=3ApmVh4kzoQVePnD2Q/rO87nbkQcMzD2k6jlEXZgHbM=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=OAMgP4BHskWNoPLVQ0buPfyxByESc4JirMUofSN57uIEb+/M86X+6taQCLrxbi7xI\n\tJCGa5zDklc2qJJ5s9QfVmB02YKTj8h/bSj1ph6xW8i9ee+6P8vPrMHjAeyl6Z/uoY8\n\tQVeH3umRkFE0qB4p99deKkaiT1eb7JZHrUDnruDs=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Kaaira Gupta <kgupta@es.iitr.ac.in>,\n\tHelen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>","Date":"Thu, 19 Mar 2020 12:52:01 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 12:52:05 -0000"}},{"id":4096,"web_url":"https://patchwork.libcamera.org/comment/4096/","msgid":"<20200319130220.GA32204@kaaira-HP-Pavilion-Notebook>","date":"2020-03-19T13:02:20","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Thu, Mar 19, 2020 at 12:52:01PM +0000, Kieran Bingham wrote:\n> Hi Kaaira,\n> \n> On 19/03/2020 12:42, Kaaira Gupta wrote:\n> > The two callers of functions libcameraPath() and isLibcameraInstalled()\n> > end up using the same process and finally use the path with\n> > libcamera.so. Hence write a function libcameraBuildPath() which\n> > combines their functions and returns the root of the build sources\n> > when the library has not been installed, but is running,thereby making\n> > call sites simpler.\n> > \n> > When the library is imstalled, libcameraBuildPath() will return an empty\n> \n> /imstalled/installed/\n> \n> If that's the only comment I'll fix while applying.\n\nDo you check this manually or is there some tool which maybe I can also\nuse from the next time so that there are no such errors?\n\n> \n> > string.\n> > \n> > Make changes in the call sites accordingly.\n> > \n> > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > ---\n> > Changes since v2:\n> > \t- rename subject line and add necessary details in commit\n> > \t  message.\n> > \t- Solve whitespace issues.\n> > \t- specify the final 'slash' for directory path in return\n> > \t  statement itself.\n> > \n> > Changes since v1:\n> >         - rename function to libcameraBuildPath() instead, and return\n> >           the root of libcamera source, and not the source directory\n> >           root.\n> >         - Return empty string instead of nullptr when ret==0\n> > \n> >  src/libcamera/include/utils.h |  2 +-\n> >  src/libcamera/ipa_manager.cpp |  5 +++--\n> >  src/libcamera/ipa_proxy.cpp   |  6 +++---\n> >  src/libcamera/utils.cpp       | 16 +++++++++-------\n> >  4 files changed, 16 insertions(+), 13 deletions(-)\n> > \n> > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> > index bc96e79..5dea8d2 100644\n> > --- a/src/libcamera/include/utils.h\n> > +++ b/src/libcamera/include/utils.h\n> > @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n> >  \n> >  bool isLibcameraInstalled();\n> >  \n> > -std::string libcameraPath();\n> > +std::string libcameraBuildPath();\n> >  \n> >  } /* namespace utils */\n> >  \n> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > index 0bd280c..bcaae35 100644\n> > --- a/src/libcamera/ipa_manager.cpp\n> > +++ b/src/libcamera/ipa_manager.cpp\n> > @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n> >  \t * path for the IPA from that point. We need to recurse one level of\n> >  \t * sub-directories to match the build tree.\n> >  \t */\n> > -\tif (!utils::isLibcameraInstalled()) {\n> > -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> > +\tstd::string root = utils::libcameraBuildPath();\n> > +\tif (!root.empty()) {\n> > +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n> >  \t\tconstexpr int maxDepth = 1;\n> >  \n> >  \t\tLOG(IPAManager, Info)\n> > diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> > index 2f866cc..5fd88a4 100644\n> > --- a/src/libcamera/ipa_proxy.cpp\n> > +++ b/src/libcamera/ipa_proxy.cpp\n> > @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> >  \t * This requires identifying the path of the libcamera.so, and\n> >  \t * referencing a relative path for the proxy workers from that point.\n> >  \t */\n> > -\tif (!utils::isLibcameraInstalled()) {\n> > -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> > -\t\t\t\t\t  + \"/proxy/worker\";\n> > +\tstd::string root = utils::libcameraBuildPath();\n> > +\tif (!root.empty()) {\n> > +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n> >  \n> >  \t\tLOG(IPAProxy, Info)\n> >  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> > index 7e118fa..42c5c76 100644\n> > --- a/src/libcamera/utils.cpp\n> > +++ b/src/libcamera/utils.cpp\n> > @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n> >   *\n> >   * \\return A string stating the path\n> >   */\n> > -std::string libcameraPath()\n> > +std::string libcameraBuildPath()\n> >  {\n> > -\tDl_info info;\n> > +\tif (!isLibcameraInstalled()) {\n> > +\t\tDl_info info;\n> >  \n> > -\t/* Look up our own symbol. */\n> > -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> > -\tif (ret == 0)\n> > -\t\treturn nullptr;\n> > +\t\t/* Look up our own symbol. */\n> > +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> > +\t\tif (ret)\n> > +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> > +\t}\n> >  \n> > -\treturn info.dli_fname;\n> > +\treturn std::string();\n> >  }\n> >  \n> >  } /* namespace utils */\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<kgupta@es.iitr.ac.in>","Received":["from mail-pl1-x642.google.com (mail-pl1-x642.google.com\n\t[IPv6:2607:f8b0:4864:20::642])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3401F629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 14:02:29 +0100 (CET)","by mail-pl1-x642.google.com with SMTP id h11so1025688plr.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 06:02:29 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.153])\n\tby smtp.gmail.com with ESMTPSA id\n\ty125sm2583566pfy.195.2020.03.19.06.02.24\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 19 Mar 2020 06:02:26 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=4qdnY0xRPPMrjeIdEK3weTyYRkfGbI4RZ2Ee+HYc+1c=;\n\tb=x+bmx3ohakg5KrfdzxDh8HokQ9k2uK8kF0iFYLXrn6Ovtu/XWn8/t56dPuziWOdPmj\n\t2ZccJT/i4bKBji/uHeC5w202fh2WiFvcyf+535kj8cQJZneLmaFAymipQW/2GylKBmRp\n\tQZ6FVE3I/Hde4HEn8VXhvAfq38eLAtTjP2CLvIEmkPG5cvDw+YIsc8PDyn+wbDfQeCTn\n\tMAN9Bw/clL3T3zTxF2Dt9C1n15oDRrmzagIe03ow2rmadGC2xbNELJUUN0Q2hUwwAPaU\n\trfEhfPLjOoxnRypUtTY2tROyuKRkSUyPj6eEstuf2xbIBR0C22zqzjysvW+D7dWRj8Ux\n\tHkkA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=4qdnY0xRPPMrjeIdEK3weTyYRkfGbI4RZ2Ee+HYc+1c=;\n\tb=J8A+9lhsmOODMd/Ke1ZMiGP88BnWE++FkE7sDNf5E+Nju9yVbZKy1tEi9zzMJ8/Pon\n\tRqqay8SLOkU9s16knera3q21zn2ny5de1Ffw219SzZ7ypXTZeNkBJ52KeQeIV7ilxpN3\n\tGiyzJxGGJVrt8oQd72/4uCuYpKTyzFhYaOdqQT1rQ1HR9JgDupQ0UQVZuSOxkPeTaEs3\n\tLQeNIYFR7flolGE+kriwKqatbZfLxktGun8RAtwGaMtg5bzjeJGR7Sko5qeiFYPg5aOK\n\tRTTTtH2T3wwFuiFW9LlKYvrwl/mSu48V+CBPlCHO5KEBUCdr8UPQNUE7QeiI8dCleq2+\n\tTAeQ==","X-Gm-Message-State":"ANhLgQ3wkAGIJbSqVuB8n6JqfMYEYKSxUncyrtQDs5hgAuVm4KO6M3KX\n\tX7RU/cpv1RGq6eThGQrAEucO0g==","X-Google-Smtp-Source":"ADFU+vudArb5s2Wyje707mWnCldedTWT68Non3l/c34xsxqsslX6uHneKgDKol+4pJG7FPDhWwQWgw==","X-Received":"by 2002:a17:902:be08:: with SMTP id\n\tr8mr3350706pls.321.1584622947329; \n\tThu, 19 Mar 2020 06:02:27 -0700 (PDT)","From":"Kaaira Gupta <kgupta@es.iitr.ac.in>","X-Google-Original-From":"Kaaira Gupta <Kaairakgupta@es.iitr.ac.in>","Date":"Thu, 19 Mar 2020 18:32:20 +0530","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>,\n\tHelen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200319130220.GA32204@kaaira-HP-Pavilion-Notebook>","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 13:02:32 -0000"}},{"id":4098,"web_url":"https://patchwork.libcamera.org/comment/4098/","msgid":"<20200319133550.GE4872@pendragon.ideasonboard.com>","date":"2020-03-19T13:35:50","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kaaira,\n\nThank you for the patch.\n\nOn Thu, Mar 19, 2020 at 06:12:52PM +0530, Kaaira Gupta wrote:\n> The two callers of functions libcameraPath() and isLibcameraInstalled()\n> end up using the same process and finally use the path with\n> libcamera.so. Hence write a function libcameraBuildPath() which\n> combines their functions and returns the root of the build sources\n> when the library has not been installed, but is running,thereby making\n\ns/running,/running, /\n\nWe don't charge extra for white space :-)\n\n> call sites simpler.\n> \n> When the library is imstalled, libcameraBuildPath() will return an empty\n\ns/imstalled/installed/\n\n> string.\n> \n> Make changes in the call sites accordingly.\n> \n> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> ---\n> Changes since v2:\n> \t- rename subject line and add necessary details in commit\n> \t  message.\n> \t- Solve whitespace issues.\n> \t- specify the final 'slash' for directory path in return\n> \t  statement itself.\n> \n> Changes since v1:\n>         - rename function to libcameraBuildPath() instead, and return\n>           the root of libcamera source, and not the source directory\n>           root.\n>         - Return empty string instead of nullptr when ret==0\n> \n>  src/libcamera/include/utils.h |  2 +-\n>  src/libcamera/ipa_manager.cpp |  5 +++--\n>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n>  src/libcamera/utils.cpp       | 16 +++++++++-------\n>  4 files changed, 16 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> index bc96e79..5dea8d2 100644\n> --- a/src/libcamera/include/utils.h\n> +++ b/src/libcamera/include/utils.h\n> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n>  \n>  bool isLibcameraInstalled();\n\nThis function isn't used externally anymore, I would drop it from the\nheader.\n\n>  \n> -std::string libcameraPath();\n> +std::string libcameraBuildPath();\n>  \n>  } /* namespace utils */\n>  \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 0bd280c..bcaae35 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n>  \t * path for the IPA from that point. We need to recurse one level of\n>  \t * sub-directories to match the build tree.\n>  \t */\n> -\tif (!utils::isLibcameraInstalled()) {\n> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> +\tstd::string root = utils::libcameraBuildPath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n\nVery neat :-)\n\n>  \t\tconstexpr int maxDepth = 1;\n>  \n>  \t\tLOG(IPAManager, Info)\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 2f866cc..5fd88a4 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n>  \t * This requires identifying the path of the libcamera.so, and\n>  \t * referencing a relative path for the proxy workers from that point.\n>  \t */\n> -\tif (!utils::isLibcameraInstalled()) {\n> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> -\t\t\t\t\t  + \"/proxy/worker\";\n> +\tstd::string root = utils::libcameraBuildPath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n>  \n>  \t\tLOG(IPAProxy, Info)\n>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> index 7e118fa..42c5c76 100644\n> --- a/src/libcamera/utils.cpp\n> +++ b/src/libcamera/utils.cpp\n> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n>   *\n>   * \\return A string stating the path\n>   */\n> -std::string libcameraPath()\n> +std::string libcameraBuildPath()\n>  {\n> -\tDl_info info;\n> +\tif (!isLibcameraInstalled()) {\n> +\t\tDl_info info;\n>  \n> -\t/* Look up our own symbol. */\n> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> -\tif (ret == 0)\n> -\t\treturn nullptr;\n> +\t\t/* Look up our own symbol. */\n> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> +\t\tif (ret)\n> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> +\t}\n>  \n> -\treturn info.dli_fname;\n> +\treturn std::string();\n\nMaybe it's my personal preference, but I find code easier to read when\ndealing away with errors immediately:\n\n\tif (isLibcameraInstalled())\n\t\treturn std::string();\n\n\tDl_info info;\n\n\t/* Look up our own symbol. */\n\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n\tif (!ret)\n\t\treturn std::string();\n\n\treturn dirname(info.dli_fname) + \"/../../\";\n\nThis way you can reduce indentation for most of the code that implements\nthe real logic.\n\nIf you're fine with this, I'm sure Kieran can change it while applying.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  }\n>  \n>  } /* namespace utils */","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C89C629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 14:35:56 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ED11DA53;\n\tThu, 19 Mar 2020 14:35:55 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584624956;\n\tbh=9O4JGKLVqhZ8ULAtQW9IA8afKAZ+dzfUc5E4K5o3wm4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WPeXR0P/hZuTRhg/GPmO/bXd8DKZLD8OYyqx+xssZeCB4vPte1NyKkmOvSOo26F49\n\tgmkiZbyp3dsObxM1C/TZA2LD7lqlGtB1txIJx7qMGypKA2g6IxIZ8KspG/Oc3Jw45v\n\twaPy5S4/yXW+9tDqVS+T0rYdl9Cg7z7oTPDpjPaI=","Date":"Thu, 19 Mar 2020 15:35:50 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kaaira Gupta <kgupta@es.iitr.ac.in>","Cc":"Helen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tkieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org","Message-ID":"<20200319133550.GE4872@pendragon.ideasonboard.com>","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 13:35:56 -0000"}},{"id":4099,"web_url":"https://patchwork.libcamera.org/comment/4099/","msgid":"<1a42f234-2db4-65cc-304b-19afd2e341c9@ideasonboard.com>","date":"2020-03-19T13:36:53","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 19/03/2020 13:02, Kaaira Gupta wrote:\n> On Thu, Mar 19, 2020 at 12:52:01PM +0000, Kieran Bingham wrote:\n>> Hi Kaaira,\n>>\n>> On 19/03/2020 12:42, Kaaira Gupta wrote:\n>>> The two callers of functions libcameraPath() and isLibcameraInstalled()\n>>> end up using the same process and finally use the path with\n>>> libcamera.so. Hence write a function libcameraBuildPath() which\n>>> combines their functions and returns the root of the build sources\n>>> when the library has not been installed, but is running,thereby making\n>>> call sites simpler.\n>>>\n>>> When the library is imstalled, libcameraBuildPath() will return an empty\n>>\n>> /imstalled/installed/\n>>\n>> If that's the only comment I'll fix while applying.\n> \n> Do you check this manually or is there some tool which maybe I can also\n> use from the next time so that there are no such errors?\n\nIn this instance it was my mail client highlighting spelling errors.\n\nI hope to be able to add spell-check support to checkstyle.py. It's\nsomething I've started, but haven't completed and the attempts I've made\nso far were really quite slow in performance :-(\n\n--\nKieran\n\n\n> \n>>\n>>> string.\n>>>\n>>> Make changes in the call sites accordingly.\n>>>\n>>> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>> ---\n>>> Changes since v2:\n>>> \t- rename subject line and add necessary details in commit\n>>> \t  message.\n>>> \t- Solve whitespace issues.\n>>> \t- specify the final 'slash' for directory path in return\n>>> \t  statement itself.\n>>>\n>>> Changes since v1:\n>>>         - rename function to libcameraBuildPath() instead, and return\n>>>           the root of libcamera source, and not the source directory\n>>>           root.\n>>>         - Return empty string instead of nullptr when ret==0\n>>>\n>>>  src/libcamera/include/utils.h |  2 +-\n>>>  src/libcamera/ipa_manager.cpp |  5 +++--\n>>>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n>>>  src/libcamera/utils.cpp       | 16 +++++++++-------\n>>>  4 files changed, 16 insertions(+), 13 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n>>> index bc96e79..5dea8d2 100644\n>>> --- a/src/libcamera/include/utils.h\n>>> +++ b/src/libcamera/include/utils.h\n>>> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n>>>  \n>>>  bool isLibcameraInstalled();\n>>>  \n>>> -std::string libcameraPath();\n>>> +std::string libcameraBuildPath();\n>>>  \n>>>  } /* namespace utils */\n>>>  \n>>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>>> index 0bd280c..bcaae35 100644\n>>> --- a/src/libcamera/ipa_manager.cpp\n>>> +++ b/src/libcamera/ipa_manager.cpp\n>>> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n>>>  \t * path for the IPA from that point. We need to recurse one level of\n>>>  \t * sub-directories to match the build tree.\n>>>  \t */\n>>> -\tif (!utils::isLibcameraInstalled()) {\n>>> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n>>> +\tstd::string root = utils::libcameraBuildPath();\n>>> +\tif (!root.empty()) {\n>>> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n>>>  \t\tconstexpr int maxDepth = 1;\n>>>  \n>>>  \t\tLOG(IPAManager, Info)\n>>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>>> index 2f866cc..5fd88a4 100644\n>>> --- a/src/libcamera/ipa_proxy.cpp\n>>> +++ b/src/libcamera/ipa_proxy.cpp\n>>> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n>>>  \t * This requires identifying the path of the libcamera.so, and\n>>>  \t * referencing a relative path for the proxy workers from that point.\n>>>  \t */\n>>> -\tif (!utils::isLibcameraInstalled()) {\n>>> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n>>> -\t\t\t\t\t  + \"/proxy/worker\";\n>>> +\tstd::string root = utils::libcameraBuildPath();\n>>> +\tif (!root.empty()) {\n>>> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n>>>  \n>>>  \t\tLOG(IPAProxy, Info)\n>>>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n>>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n>>> index 7e118fa..42c5c76 100644\n>>> --- a/src/libcamera/utils.cpp\n>>> +++ b/src/libcamera/utils.cpp\n>>> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n>>>   *\n>>>   * \\return A string stating the path\n>>>   */\n>>> -std::string libcameraPath()\n>>> +std::string libcameraBuildPath()\n>>>  {\n>>> -\tDl_info info;\n>>> +\tif (!isLibcameraInstalled()) {\n>>> +\t\tDl_info info;\n>>>  \n>>> -\t/* Look up our own symbol. */\n>>> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n>>> -\tif (ret == 0)\n>>> -\t\treturn nullptr;\n>>> +\t\t/* Look up our own symbol. */\n>>> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n>>> +\t\tif (ret)\n>>> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n>>> +\t}\n>>>  \n>>> -\treturn info.dli_fname;\n>>> +\treturn std::string();\n>>>  }\n>>>  \n>>>  } /* namespace utils */\n>>>\n>>\n>> -- \n>> Regards\n>> --\n>> Kieran","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2237629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 14:36:56 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 678D0A53;\n\tThu, 19 Mar 2020 14:36:56 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584625016;\n\tbh=YlYzAdYV5jimi7y/55xtg0y4u0a3MKUKSLOXowGGr+8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=udqVJGCKh1BDRgcEcYL+I0RJ/2sVwOme6gBPQvWjPWSLWiZAEhZDZ+++IyiuW5sS9\n\tPgfBYvG0H+ysahX4qXKCbVJNbdEa6ax11eTc1Rsb53CsDCjIT+goEeIQwARX+73jxh\n\t9Yga9XxRcQRN4ekN6kcfBz8UtQrRY6Tb20dtm4ys=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Kaaira Gupta <kgupta@es.iitr.ac.in>","Cc":"Helen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>\n\t<20200319130220.GA32204@kaaira-HP-Pavilion-Notebook>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<1a42f234-2db4-65cc-304b-19afd2e341c9@ideasonboard.com>","Date":"Thu, 19 Mar 2020 13:36:53 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200319130220.GA32204@kaaira-HP-Pavilion-Notebook>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 13:36:57 -0000"}},{"id":4100,"web_url":"https://patchwork.libcamera.org/comment/4100/","msgid":"<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","date":"2020-03-19T14:05:23","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent, Kaaira,\n\nOn 19/03/2020 13:35, Laurent Pinchart wrote:\n> Hi Kaaira,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 19, 2020 at 06:12:52PM +0530, Kaaira Gupta wrote:\n>> The two callers of functions libcameraPath() and isLibcameraInstalled()\n>> end up using the same process and finally use the path with\n>> libcamera.so. Hence write a function libcameraBuildPath() which\n>> combines their functions and returns the root of the build sources\n>> when the library has not been installed, but is running,thereby making\n> \n> s/running,/running, /\n> \n> We don't charge extra for white space :-)\n\nOoops I missed that one.\n\n> \n>> call sites simpler.\n>>\n>> When the library is imstalled, libcameraBuildPath() will return an empty\n> \n> s/imstalled/installed/\n> \n>> string.\n>>\n>> Make changes in the call sites accordingly.\n>>\n>> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n>> ---\n>> Changes since v2:\n>> \t- rename subject line and add necessary details in commit\n>> \t  message.\n>> \t- Solve whitespace issues.\n>> \t- specify the final 'slash' for directory path in return\n>> \t  statement itself.\n>>\n>> Changes since v1:\n>>         - rename function to libcameraBuildPath() instead, and return\n>>           the root of libcamera source, and not the source directory\n>>           root.\n>>         - Return empty string instead of nullptr when ret==0\n>>\n>>  src/libcamera/include/utils.h |  2 +-\n>>  src/libcamera/ipa_manager.cpp |  5 +++--\n>>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n>>  src/libcamera/utils.cpp       | 16 +++++++++-------\n>>  4 files changed, 16 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n>> index bc96e79..5dea8d2 100644\n>> --- a/src/libcamera/include/utils.h\n>> +++ b/src/libcamera/include/utils.h\n>> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n>>  \n>>  bool isLibcameraInstalled();\n> \n> This function isn't used externally anymore, I would drop it from the\n> header.\n\nbut it /could/ be used. It's not a static, and other locations might\nwant to be able to detect this in the future.\n\nThat was sort of the reason of keeping the two functions separate rather\nthan merging them.\n\nStill it could easily be added back in if needed in the future too ...\n\n>>  \n>> -std::string libcameraPath();\n>> +std::string libcameraBuildPath();\n>>  \n>>  } /* namespace utils */\n>>  \n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 0bd280c..bcaae35 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n>>  \t * path for the IPA from that point. We need to recurse one level of\n>>  \t * sub-directories to match the build tree.\n>>  \t */\n>> -\tif (!utils::isLibcameraInstalled()) {\n>> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n>> +\tstd::string root = utils::libcameraBuildPath();\n>> +\tif (!root.empty()) {\n>> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n> \n> Very neat :-)\n> \n>>  \t\tconstexpr int maxDepth = 1;\n>>  \n>>  \t\tLOG(IPAManager, Info)\n>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>> index 2f866cc..5fd88a4 100644\n>> --- a/src/libcamera/ipa_proxy.cpp\n>> +++ b/src/libcamera/ipa_proxy.cpp\n>> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n>>  \t * This requires identifying the path of the libcamera.so, and\n>>  \t * referencing a relative path for the proxy workers from that point.\n>>  \t */\n>> -\tif (!utils::isLibcameraInstalled()) {\n>> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n>> -\t\t\t\t\t  + \"/proxy/worker\";\n>> +\tstd::string root = utils::libcameraBuildPath();\n>> +\tif (!root.empty()) {\n>> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n>>  \n>>  \t\tLOG(IPAProxy, Info)\n>>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n>> index 7e118fa..42c5c76 100644\n>> --- a/src/libcamera/utils.cpp\n>> +++ b/src/libcamera/utils.cpp\n>> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n>>   *\n>>   * \\return A string stating the path\n>>   */\n>> -std::string libcameraPath()\n>> +std::string libcameraBuildPath()\n>>  {\n>> -\tDl_info info;\n>> +\tif (!isLibcameraInstalled()) {\n>> +\t\tDl_info info;\n>>  \n>> -\t/* Look up our own symbol. */\n>> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n>> -\tif (ret == 0)\n>> -\t\treturn nullptr;\n>> +\t\t/* Look up our own symbol. */\n>> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n>> +\t\tif (ret)\n>> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n>> +\t}\n>>  \n>> -\treturn info.dli_fname;\n>> +\treturn std::string();\n> \n> Maybe it's my personal preference, but I find code easier to read when\n> dealing away with errors immediately:\n> \n> \tif (isLibcameraInstalled())\n> \t\treturn std::string();\n> \n> \tDl_info info;\n> \n> \t/* Look up our own symbol. */\n> \tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> \tif (!ret)\n> \t\treturn std::string();\n> \n> \treturn dirname(info.dli_fname) + \"/../../\";\n> \n> This way you can reduce indentation for most of the code that implements\n> the real logic.\n> \n> If you're fine with this, I'm sure Kieran can change it while applying.\n\nHrm, I usually look to reduce the number of return paths ... but I'm not\nfussed either way. It's only because I had to work on MISRA projects in\nthe past.\n\nKaaira - up to you really I think. What do you prefer? If you like\nLaurent's version - please send a v4 with the other fixes and I can test\nand apply it.\n\n--\nKieran\n\n\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n>>  }\n>>  \n>>  } /* namespace utils */\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0462629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 15:05:27 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BEDE3A53;\n\tThu, 19 Mar 2020 15:05:26 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584626727;\n\tbh=/JYtYo6EQp7N4F9lE05pt1MgbSPbhlnO8Sv9zRFhl5c=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=f3MXx+7YY7lwuQCWXx7Ki/Xt3tzeKLvTET0GyXLOajzgp5dplDEPJMyzpMFSMBB3U\n\tXo6rfzV0RkTRHv3XpgS5ekh7bZlT6zhmT8KtLWVB/ACfkbHjjhv/2KPBAa8RjEvNME\n\thnWTKUuFyUu6HM1uDFyWgYPxBBb9BLvrcEGsGuPI=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKaaira Gupta <kgupta@es.iitr.ac.in>","Cc":"Helen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<20200319133550.GE4872@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","Date":"Thu, 19 Mar 2020 14:05:23 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200319133550.GE4872@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 14:05:27 -0000"}},{"id":4101,"web_url":"https://patchwork.libcamera.org/comment/4101/","msgid":"<20200319144349.GA3045@kaaira-HP-Pavilion-Notebook>","date":"2020-03-19T14:43:49","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Thu, Mar 19, 2020 at 02:05:23PM +0000, Kieran Bingham wrote:\n> Hi Laurent, Kaaira,\n> \n> On 19/03/2020 13:35, Laurent Pinchart wrote:\n> > Hi Kaaira,\n> > \n> > Thank you for the patch.\n> > \n> > On Thu, Mar 19, 2020 at 06:12:52PM +0530, Kaaira Gupta wrote:\n> >> The two callers of functions libcameraPath() and isLibcameraInstalled()\n> >> end up using the same process and finally use the path with\n> >> libcamera.so. Hence write a function libcameraBuildPath() which\n> >> combines their functions and returns the root of the build sources\n> >> when the library has not been installed, but is running,thereby making\n> > \n> > s/running,/running, /\n> > \n> > We don't charge extra for white space :-)\n> \n> Ooops I missed that one.\n> \n> > \n> >> call sites simpler.\n> >>\n> >> When the library is imstalled, libcameraBuildPath() will return an empty\n> > \n> > s/imstalled/installed/\n> > \n> >> string.\n> >>\n> >> Make changes in the call sites accordingly.\n> >>\n> >> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> >> ---\n> >> Changes since v2:\n> >> \t- rename subject line and add necessary details in commit\n> >> \t  message.\n> >> \t- Solve whitespace issues.\n> >> \t- specify the final 'slash' for directory path in return\n> >> \t  statement itself.\n> >>\n> >> Changes since v1:\n> >>         - rename function to libcameraBuildPath() instead, and return\n> >>           the root of libcamera source, and not the source directory\n> >>           root.\n> >>         - Return empty string instead of nullptr when ret==0\n> >>\n> >>  src/libcamera/include/utils.h |  2 +-\n> >>  src/libcamera/ipa_manager.cpp |  5 +++--\n> >>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n> >>  src/libcamera/utils.cpp       | 16 +++++++++-------\n> >>  4 files changed, 16 insertions(+), 13 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> >> index bc96e79..5dea8d2 100644\n> >> --- a/src/libcamera/include/utils.h\n> >> +++ b/src/libcamera/include/utils.h\n> >> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n> >>  \n> >>  bool isLibcameraInstalled();\n> > \n> > This function isn't used externally anymore, I would drop it from the\n> > header.\n> \n> but it /could/ be used. It's not a static, and other locations might\n> want to be able to detect this in the future.\n> \n> That was sort of the reason of keeping the two functions separate rather\n> than merging them.\n> \n> Still it could easily be added back in if needed in the future too ...\n\nYes, we can drop it for now. I'll do that.\n\n> \n> >>  \n> >> -std::string libcameraPath();\n> >> +std::string libcameraBuildPath();\n> >>  \n> >>  } /* namespace utils */\n> >>  \n> >> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> >> index 0bd280c..bcaae35 100644\n> >> --- a/src/libcamera/ipa_manager.cpp\n> >> +++ b/src/libcamera/ipa_manager.cpp\n> >> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n> >>  \t * path for the IPA from that point. We need to recurse one level of\n> >>  \t * sub-directories to match the build tree.\n> >>  \t */\n> >> -\tif (!utils::isLibcameraInstalled()) {\n> >> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> >> +\tstd::string root = utils::libcameraBuildPath();\n> >> +\tif (!root.empty()) {\n> >> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n> > \n> > Very neat :-)\n> > \n> >>  \t\tconstexpr int maxDepth = 1;\n> >>  \n> >>  \t\tLOG(IPAManager, Info)\n> >> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> >> index 2f866cc..5fd88a4 100644\n> >> --- a/src/libcamera/ipa_proxy.cpp\n> >> +++ b/src/libcamera/ipa_proxy.cpp\n> >> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> >>  \t * This requires identifying the path of the libcamera.so, and\n> >>  \t * referencing a relative path for the proxy workers from that point.\n> >>  \t */\n> >> -\tif (!utils::isLibcameraInstalled()) {\n> >> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> >> -\t\t\t\t\t  + \"/proxy/worker\";\n> >> +\tstd::string root = utils::libcameraBuildPath();\n> >> +\tif (!root.empty()) {\n> >> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n> >>  \n> >>  \t\tLOG(IPAProxy, Info)\n> >>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> >> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> >> index 7e118fa..42c5c76 100644\n> >> --- a/src/libcamera/utils.cpp\n> >> +++ b/src/libcamera/utils.cpp\n> >> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n> >>   *\n> >>   * \\return A string stating the path\n> >>   */\n> >> -std::string libcameraPath()\n> >> +std::string libcameraBuildPath()\n> >>  {\n> >> -\tDl_info info;\n> >> +\tif (!isLibcameraInstalled()) {\n> >> +\t\tDl_info info;\n> >>  \n> >> -\t/* Look up our own symbol. */\n> >> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> >> -\tif (ret == 0)\n> >> -\t\treturn nullptr;\n> >> +\t\t/* Look up our own symbol. */\n> >> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> >> +\t}\n> >>  \n> >> -\treturn info.dli_fname;\n> >> +\treturn std::string();\n> > \n> > Maybe it's my personal preference, but I find code easier to read when\n> > dealing away with errors immediately:\n> > \n> > \tif (isLibcameraInstalled())\n> > \t\treturn std::string();\n> > \n> > \tDl_info info;\n> > \n> > \t/* Look up our own symbol. */\n> > \tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> > \tif (!ret)\n> > \t\treturn std::string();\n> > \n> > \treturn dirname(info.dli_fname) + \"/../../\";\n> > \n> > This way you can reduce indentation for most of the code that implements\n> > the real logic.\n> > \n> > If you're fine with this, I'm sure Kieran can change it while applying.\n> \n> Hrm, I usually look to reduce the number of return paths ... but I'm not\n> fussed either way. It's only because I had to work on MISRA projects in\n> the past.\n> \n> Kaaira - up to you really I think. What do you prefer? If you like\n> Laurent's version - please send a v4 with the other fixes and I can test\n> and apply it.\n\nI think I should change it, that way the code looks cleaner. I'll send\nthe updated version.\n\n> \n> --\n> Kieran\n> \n> \n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >>  }\n> >>  \n> >>  } /* namespace utils */\n> >","headers":{"Return-Path":"<kgupta@es.iitr.ac.in>","Received":["from mail-pg1-x543.google.com (mail-pg1-x543.google.com\n\t[IPv6:2607:f8b0:4864:20::543])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB98F629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 15:43:58 +0100 (CET)","by mail-pg1-x543.google.com with SMTP id t3so1406724pgn.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 07:43:58 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.153])\n\tby smtp.gmail.com with ESMTPSA id\n\tu6sm2640709pgj.7.2020.03.19.07.43.54\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 19 Mar 2020 07:43:56 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=bYg6RK3Z5hOnywNQW20d54VxGoaGqkZwexhMz0l46NU=;\n\tb=aNHRKeQwM7xEnvK8s97y11tDGV9gy+YJlT5c1twJWItq5OLeXn69CAiUneNmR8tc0t\n\t4mZ4z/5o3ZeWBga5veyZU1dk/zLc0ReKF1zKFzecZepx1C2q4GdXjwr9yWcCEWYgqqjC\n\tNzKM5RLDBYcthlJhtpJA9B8Toxus5oo71QTVkcE0mAlXonk/AXNtTewOiUYwtzU3Q2Xf\n\tmOXuB8mxmGEeQIW/Xb0HqKovUUp5nL5rBSAcHu/DTo1WEC0CXhZTPdZDwpIkfW6Q2p/E\n\tunHnHFIycRpumdXEZQmLZWvUyWTBU0ZH//ztskdHuHCWlx4+e8SG3ylhg+M2a1UotSyI\n\tHRzg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=bYg6RK3Z5hOnywNQW20d54VxGoaGqkZwexhMz0l46NU=;\n\tb=ZR5E7vr7Ua3jUg/zFsbbHWZqiuWVBShdNy3WL7WsUVzs9DGSsWPOdtTu3uublAgKAC\n\t0trBrB7MQbA9RpLiESh2pynk1wmZV5BGT0w2/RM24IFmA4xIke07RO7y+2wbQ9wfvTYM\n\tHb03towzILG4wjE/vRjO6yOJG3wb0io3/gaDTcXmzzkalvtLrB8AqYX8UUfGlpYOoVkG\n\t/Lct62/nJoHLLghsKNOzN4s81oV+6ygVEMl/jOn4MyDXITXFuVfaRauUdFg5MT4c+Qxs\n\tmVpcRjhwbLhG90d30SJHk3I6q7L+TciY1LbrwwC/crWqcYLlGPXt662ypzOlIcJ+jTXI\n\tRF1A==","X-Gm-Message-State":"ANhLgQ32+PIqbrX7oM/N22SBiy+BQ91XN/V6SYBvZBfUvxEEmuXIyqL+\n\tjgxYCXoH7mwkYOrRlaC1kniGkVS6nJI=","X-Google-Smtp-Source":"ADFU+vtJ4quJaLqFIVo+n9PGsVzo+PDf29guSCKlD5DGMJQvBLlVZMlgRrQe8Z0lsP7RfE5q0shixQ==","X-Received":"by 2002:a65:648e:: with SMTP id\n\te14mr3600565pgv.182.1584629037341; \n\tThu, 19 Mar 2020 07:43:57 -0700 (PDT)","From":"Kaaira Gupta <kgupta@es.iitr.ac.in>","X-Google-Original-From":"Kaaira Gupta <Kaairakgupta@es.iitr.ac.in>","Date":"Thu, 19 Mar 2020 20:13:49 +0530","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKaaira Gupta <kgupta@es.iitr.ac.in>,\n\tHelen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200319144349.GA3045@kaaira-HP-Pavilion-Notebook>","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<20200319133550.GE4872@pendragon.ideasonboard.com>\n\t<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 14:43:59 -0000"}},{"id":4102,"web_url":"https://patchwork.libcamera.org/comment/4102/","msgid":"<20200319144542.GA14585@pendragon.ideasonboard.com>","date":"2020-03-19T14:45:42","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Mar 19, 2020 at 02:05:23PM +0000, Kieran Bingham wrote:\n> On 19/03/2020 13:35, Laurent Pinchart wrote:\n> > On Thu, Mar 19, 2020 at 06:12:52PM +0530, Kaaira Gupta wrote:\n> >> The two callers of functions libcameraPath() and isLibcameraInstalled()\n> >> end up using the same process and finally use the path with\n> >> libcamera.so. Hence write a function libcameraBuildPath() which\n> >> combines their functions and returns the root of the build sources\n> >> when the library has not been installed, but is running,thereby making\n> > \n> > s/running,/running, /\n> > \n> > We don't charge extra for white space :-)\n> \n> Ooops I missed that one.\n> \n> >> call sites simpler.\n> >>\n> >> When the library is imstalled, libcameraBuildPath() will return an empty\n> > \n> > s/imstalled/installed/\n> > \n> >> string.\n> >>\n> >> Make changes in the call sites accordingly.\n> >>\n> >> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> >> ---\n> >> Changes since v2:\n> >> \t- rename subject line and add necessary details in commit\n> >> \t  message.\n> >> \t- Solve whitespace issues.\n> >> \t- specify the final 'slash' for directory path in return\n> >> \t  statement itself.\n> >>\n> >> Changes since v1:\n> >>         - rename function to libcameraBuildPath() instead, and return\n> >>           the root of libcamera source, and not the source directory\n> >>           root.\n> >>         - Return empty string instead of nullptr when ret==0\n> >>\n> >>  src/libcamera/include/utils.h |  2 +-\n> >>  src/libcamera/ipa_manager.cpp |  5 +++--\n> >>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n> >>  src/libcamera/utils.cpp       | 16 +++++++++-------\n> >>  4 files changed, 16 insertions(+), 13 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> >> index bc96e79..5dea8d2 100644\n> >> --- a/src/libcamera/include/utils.h\n> >> +++ b/src/libcamera/include/utils.h\n> >> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n> >>  \n> >>  bool isLibcameraInstalled();\n> > \n> > This function isn't used externally anymore, I would drop it from the\n> > header.\n> \n> but it /could/ be used. It's not a static, and other locations might\n> want to be able to detect this in the future.\n> \n> That was sort of the reason of keeping the two functions separate rather\n> than merging them.\n\nI think I'd rather not have callers check if libcamera is installed, but\ninstead go for a bit of a higher level API where callers get the\ninformation they need, such as the build root directory.\n\n> Still it could easily be added back in if needed in the future too ...\n\nThat too, yes.\n\n> >>  \n> >> -std::string libcameraPath();\n> >> +std::string libcameraBuildPath();\n> >>  \n> >>  } /* namespace utils */\n> >>  \n> >> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> >> index 0bd280c..bcaae35 100644\n> >> --- a/src/libcamera/ipa_manager.cpp\n> >> +++ b/src/libcamera/ipa_manager.cpp\n> >> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n> >>  \t * path for the IPA from that point. We need to recurse one level of\n> >>  \t * sub-directories to match the build tree.\n> >>  \t */\n> >> -\tif (!utils::isLibcameraInstalled()) {\n> >> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> >> +\tstd::string root = utils::libcameraBuildPath();\n> >> +\tif (!root.empty()) {\n> >> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n> > \n> > Very neat :-)\n> > \n> >>  \t\tconstexpr int maxDepth = 1;\n> >>  \n> >>  \t\tLOG(IPAManager, Info)\n> >> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> >> index 2f866cc..5fd88a4 100644\n> >> --- a/src/libcamera/ipa_proxy.cpp\n> >> +++ b/src/libcamera/ipa_proxy.cpp\n> >> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> >>  \t * This requires identifying the path of the libcamera.so, and\n> >>  \t * referencing a relative path for the proxy workers from that point.\n> >>  \t */\n> >> -\tif (!utils::isLibcameraInstalled()) {\n> >> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> >> -\t\t\t\t\t  + \"/proxy/worker\";\n> >> +\tstd::string root = utils::libcameraBuildPath();\n> >> +\tif (!root.empty()) {\n> >> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n> >>  \n> >>  \t\tLOG(IPAProxy, Info)\n> >>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> >> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> >> index 7e118fa..42c5c76 100644\n> >> --- a/src/libcamera/utils.cpp\n> >> +++ b/src/libcamera/utils.cpp\n> >> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n> >>   *\n> >>   * \\return A string stating the path\n> >>   */\n> >> -std::string libcameraPath()\n> >> +std::string libcameraBuildPath()\n> >>  {\n> >> -\tDl_info info;\n> >> +\tif (!isLibcameraInstalled()) {\n> >> +\t\tDl_info info;\n> >>  \n> >> -\t/* Look up our own symbol. */\n> >> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> >> -\tif (ret == 0)\n> >> -\t\treturn nullptr;\n> >> +\t\t/* Look up our own symbol. */\n> >> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> >> +\t}\n> >>  \n> >> -\treturn info.dli_fname;\n> >> +\treturn std::string();\n> > \n> > Maybe it's my personal preference, but I find code easier to read when\n> > dealing away with errors immediately:\n> > \n> > \tif (isLibcameraInstalled())\n> > \t\treturn std::string();\n> > \n> > \tDl_info info;\n> > \n> > \t/* Look up our own symbol. */\n> > \tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> > \tif (!ret)\n> > \t\treturn std::string();\n> > \n> > \treturn dirname(info.dli_fname) + \"/../../\";\n> > \n> > This way you can reduce indentation for most of the code that implements\n> > the real logic.\n> > \n> > If you're fine with this, I'm sure Kieran can change it while applying.\n> \n> Hrm, I usually look to reduce the number of return paths ... but I'm not\n> fussed either way. It's only because I had to work on MISRA projects in\n> the past.\n\nSo less return paths are considered to lead to less bugs ? Interesting\nconcept, but I'm not sure how true it is :-)\n\n> Kaaira - up to you really I think. What do you prefer? If you like\n> Laurent's version - please send a v4 with the other fixes and I can test\n> and apply it.\n> \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >>  }\n> >>  \n> >>  } /* namespace utils */","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4100A629AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 15:45:48 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98C64A53;\n\tThu, 19 Mar 2020 15:45:47 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584629147;\n\tbh=eig7EteeICZGFrG6+w1f+jYiwS0rAwrpvozVTKVK03Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ooPhd4rOagsQuGnElY3tO9jLTuzH81VsmtZBBQ1fVc30NPidCDvj340FPnSG3V9q0\n\tS3yUWZNrNSRXB2f1G3A1RVL8VopvQs2SffVXuveOes70KnJFKPb400Cs/opJrooJMY\n\tiJasTj+s6ge4hRtvjA26OE0sgInsFPOMKqN0aWrU=","Date":"Thu, 19 Mar 2020 16:45:42 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>,\n\tHelen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200319144542.GA14585@pendragon.ideasonboard.com>","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<20200319133550.GE4872@pendragon.ideasonboard.com>\n\t<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<3f437ed0-82c8-f019-9e40-5742ebd18168@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 14:45:48 -0000"}},{"id":4107,"web_url":"https://patchwork.libcamera.org/comment/4107/","msgid":"<20200319163500.GA6536@kaaira-HP-Pavilion-Notebook>","date":"2020-03-19T16:35:00","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Thu, Mar 19, 2020 at 01:36:53PM +0000, Kieran Bingham wrote:\n> On 19/03/2020 13:02, Kaaira Gupta wrote:\n> > On Thu, Mar 19, 2020 at 12:52:01PM +0000, Kieran Bingham wrote:\n> >> Hi Kaaira,\n> >>\n> >> On 19/03/2020 12:42, Kaaira Gupta wrote:\n> >>> The two callers of functions libcameraPath() and isLibcameraInstalled()\n> >>> end up using the same process and finally use the path with\n> >>> libcamera.so. Hence write a function libcameraBuildPath() which\n> >>> combines their functions and returns the root of the build sources\n> >>> when the library has not been installed, but is running,thereby making\n> >>> call sites simpler.\n> >>>\n> >>> When the library is imstalled, libcameraBuildPath() will return an empty\n> >>\n> >> /imstalled/installed/\n> >>\n> >> If that's the only comment I'll fix while applying.\n> > \n> > Do you check this manually or is there some tool which maybe I can also\n> > use from the next time so that there are no such errors?\n> \n> In this instance it was my mail client highlighting spelling errors.\n> \n> I hope to be able to add spell-check support to checkstyle.py. It's\n> something I've started, but haven't completed and the attempts I've made\n> so far were really quite slow in performance :-(\n\nCan't the checkpatch.pl script used in kernel be used for this as well?\nIt almost checks everything\n\n> \n> --\n> Kieran\n> \n> \n> > \n> >>\n> >>> string.\n> >>>\n> >>> Make changes in the call sites accordingly.\n> >>>\n> >>> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> >>\n> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >>\n> >>> ---\n> >>> Changes since v2:\n> >>> \t- rename subject line and add necessary details in commit\n> >>> \t  message.\n> >>> \t- Solve whitespace issues.\n> >>> \t- specify the final 'slash' for directory path in return\n> >>> \t  statement itself.\n> >>>\n> >>> Changes since v1:\n> >>>         - rename function to libcameraBuildPath() instead, and return\n> >>>           the root of libcamera source, and not the source directory\n> >>>           root.\n> >>>         - Return empty string instead of nullptr when ret==0\n> >>>\n> >>>  src/libcamera/include/utils.h |  2 +-\n> >>>  src/libcamera/ipa_manager.cpp |  5 +++--\n> >>>  src/libcamera/ipa_proxy.cpp   |  6 +++---\n> >>>  src/libcamera/utils.cpp       | 16 +++++++++-------\n> >>>  4 files changed, 16 insertions(+), 13 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h\n> >>> index bc96e79..5dea8d2 100644\n> >>> --- a/src/libcamera/include/utils.h\n> >>> +++ b/src/libcamera/include/utils.h\n> >>> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim);\n> >>>  \n> >>>  bool isLibcameraInstalled();\n> >>>  \n> >>> -std::string libcameraPath();\n> >>> +std::string libcameraBuildPath();\n> >>>  \n> >>>  } /* namespace utils */\n> >>>  \n> >>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> >>> index 0bd280c..bcaae35 100644\n> >>> --- a/src/libcamera/ipa_manager.cpp\n> >>> +++ b/src/libcamera/ipa_manager.cpp\n> >>> @@ -119,8 +119,9 @@ IPAManager::IPAManager()\n> >>>  \t * path for the IPA from that point. We need to recurse one level of\n> >>>  \t * sub-directories to match the build tree.\n> >>>  \t */\n> >>> -\tif (!utils::isLibcameraInstalled()) {\n> >>> -\t\tstd::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + \"/../ipa\";\n> >>> +\tstd::string root = utils::libcameraBuildPath();\n> >>> +\tif (!root.empty()) {\n> >>> +\t\tstd::string ipaBuildPath = root + \"src/ipa\";\n> >>>  \t\tconstexpr int maxDepth = 1;\n> >>>  \n> >>>  \t\tLOG(IPAManager, Info)\n> >>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> >>> index 2f866cc..5fd88a4 100644\n> >>> --- a/src/libcamera/ipa_proxy.cpp\n> >>> +++ b/src/libcamera/ipa_proxy.cpp\n> >>> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> >>>  \t * This requires identifying the path of the libcamera.so, and\n> >>>  \t * referencing a relative path for the proxy workers from that point.\n> >>>  \t */\n> >>> -\tif (!utils::isLibcameraInstalled()) {\n> >>> -\t\tstd::string ipaProxyDir = utils::dirname(utils::libcameraPath())\n> >>> -\t\t\t\t\t  + \"/proxy/worker\";\n> >>> +\tstd::string root = utils::libcameraBuildPath();\n> >>> +\tif (!root.empty()) {\n> >>> +\t\tstd::string ipaProxyDir = root + \"src/libcamera/proxy/worker\";\n> >>>  \n> >>>  \t\tLOG(IPAProxy, Info)\n> >>>  \t\t\t<< \"libcamera is not installed. Loading proxy workers from'\"\n> >>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> >>> index 7e118fa..42c5c76 100644\n> >>> --- a/src/libcamera/utils.cpp\n> >>> +++ b/src/libcamera/utils.cpp\n> >>> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()\n> >>>   *\n> >>>   * \\return A string stating the path\n> >>>   */\n> >>> -std::string libcameraPath()\n> >>> +std::string libcameraBuildPath()\n> >>>  {\n> >>> -\tDl_info info;\n> >>> +\tif (!isLibcameraInstalled()) {\n> >>> +\t\tDl_info info;\n> >>>  \n> >>> -\t/* Look up our own symbol. */\n> >>> -\tint ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> >>> -\tif (ret == 0)\n> >>> -\t\treturn nullptr;\n> >>> +\t\t/* Look up our own symbol. */\n> >>> +\t\tint ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);\n> >>> +\t\tif (ret)\n> >>> +\t\t\treturn dirname(info.dli_fname) + \"/../../\";\n> >>> +\t}\n> >>>  \n> >>> -\treturn info.dli_fname;\n> >>> +\treturn std::string();\n> >>>  }\n> >>>  \n> >>>  } /* namespace utils */\n> >>>\n> >>\n> >> -- \n> >> Regards\n> >> --\n> >> Kieran\n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<kgupta@es.iitr.ac.in>","Received":["from mail-pf1-x441.google.com (mail-pf1-x441.google.com\n\t[IPv6:2607:f8b0:4864:20::441])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 77D8660418\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 17:35:08 +0100 (CET)","by mail-pf1-x441.google.com with SMTP id x2so1695983pfn.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 09:35:08 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.153])\n\tby smtp.gmail.com with ESMTPSA id\n\tb25sm2988861pfd.185.2020.03.19.09.35.04\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 19 Mar 2020 09:35:06 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=8aksp6kHuHoCuUIOPIjC/EfN/F9nu/HdIBDaRheZyX4=;\n\tb=BwxdKCJ+NywjoCR1lhfPnWI6T2/duUESnXGfAvEQZwFJPB0Tgz3DeJ9LRyDKAXIKZw\n\tfBnbnIHegzskLLsvrOAnP6iGcOQNj6vVGTXRQw0kcE3WGOK7Fn84ARXooEsLdjSHAXea\n\t25NRzZECI+dCIGrc+aUtU6ZcZ0NKBaXAwikixzg4VqEMp5gEH2aA6VHJCmuuUZQ/5tZX\n\tyoSvrZUaFPKcuvqRtAE7XYFXmYCOjPoPJMiA+UhJNv5h6ThTktDFQSVWvtisb7ZkAIWY\n\toHpGfGR5jx9Rau0bexiX6CYE/VgOBIlurHyARs6HzKkZkTm1+XDDOVPqw9EriGpHCjrQ\n\tUeUw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=8aksp6kHuHoCuUIOPIjC/EfN/F9nu/HdIBDaRheZyX4=;\n\tb=H4cOK3nozYau+S7UUoeLRL0jQCfdYWRmwXJcYMxHCVgglqsHy1nuBH4QGNwJ7sgnao\n\tt/eXqdwr5oaFkixukDKLbcU3q+UmQdulnFTvO2qEm7uW1B/cMPqjoTeUkrCqVtUtzonu\n\tqLoto4yJWPm4NoV+4V/z10C8pCyyDzGIz4pHhUfSKyyx8GfACwlrdhI2+/o04Ek2+NNx\n\tUy64Boj6BfVV/RTZma4nk8vcSeFMqlI6vo2czk9UCGNH6FftrHWHMwn87ppkCp4hylqX\n\tzu6Y7HvWJ7+XLqwu4K9S/n2uiYjn2/AQ+ACGzSD25AL3HigPYs9sayxorWnqtibDf+Rt\n\t8IzQ==","X-Gm-Message-State":"ANhLgQ3w26gTRwJvAUmtZD2U+2bBCLUacIKiSsXX2hRclpauG+VsY2zR\n\toWmDW5mH2vDrD0b8RmodG9ToCuTygTc=","X-Google-Smtp-Source":"ADFU+vsFH/LlMzFQQO9zKYiA3vJ69+VQLtheLk2+4jgAmtVM09wAnwuBognYYagE6sqlEkpPD7Ch2w==","X-Received":"by 2002:a63:e544:: with SMTP id z4mr4199541pgj.174.1584635706813;\n\tThu, 19 Mar 2020 09:35:06 -0700 (PDT)","From":"Kaaira Gupta <kgupta@es.iitr.ac.in>","X-Google-Original-From":"Kaaira Gupta <Kaairakgupta@es.iitr.ac.in>","Date":"Thu, 19 Mar 2020 22:05:00 +0530","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>,\n\tHelen Koike <helen.koike@collabora.com>,\n\tVaishali Thakkar <vthakkar@vaishalithakkar.in>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200319163500.GA6536@kaaira-HP-Pavilion-Notebook>","References":"<20200319124252.GA31289@kaaira-HP-Pavilion-Notebook>\n\t<26793a0a-86f4-1e72-0cde-4a4afde0044c@ideasonboard.com>\n\t<20200319130220.GA32204@kaaira-HP-Pavilion-Notebook>\n\t<1a42f234-2db4-65cc-304b-19afd2e341c9@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1a42f234-2db4-65cc-304b-19afd2e341c9@ideasonboard.com>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA v3] libcamera: utils: adapt\n\tlibcameraPath to match use cases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 19 Mar 2020 16:35:11 -0000"}}]