[{"id":1778,"web_url":"https://patchwork.libcamera.org/comment/1778/","msgid":"<20190606094154.GC12825@pendragon.ideasonboard.com>","date":"2019-06-06T09:41:54","subject":"Re: [libcamera-devel] [RFC PATCH 03/10] libcamera: ipa_module: add\n\tloading error messages","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jun 05, 2019 at 06:18:10PM -0400, Paul Elder wrote:\n> Currently, if an IPAModule fails to be constructed due to size mismatch\n> of struct IPAModuleInfo, the error is simply \"symbol not found\". Add an\n> error message to say that the symbol is found, but not valid.\n> \n> Also add an error message to tell, if an IPA module failed to load, the\n> path to the IPA module shared object that was attempted to be loaded.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/libcamera/ipa_module.cpp | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n> \n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index e1d4b27..84c77f7 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -145,6 +145,12 @@ int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,\n>  \t\t\ttargetSymbol = sym;\n>  \t\t\tbreak;\n>  \t\t}\n> +\n> +\t\tif (!strcmp(name, symbol)) {\n> +\t\t\tLOG(IPAModule, Error)\n> +\t\t\t\t<< \"Symbol \" << symbol\n> +\t\t\t\t<< \" found, but not valid. Check module version.\";\n> +\t\t}\n\nI'd rather not mention module version here, as elfLoadSymbol() is a\ngeneric function, not specific to modules (we may reuse it later for\nother ELF parsing needs). That's why I think it would be best for\nelfLoadSymbol() to return both the pointer and the size, which can then\nbe checked in the caller with module-aware code. Furthermore, if we bump\nthe module info version later and need to implement backward\ncompatibility, it will be easier to load the symbol and then check for\nthe different supported sizes in the caller.\n\n>  \t}\n>  \n>  \tif (targetSymbol == nullptr) {\n> @@ -286,6 +292,12 @@ int IPAModule::loadIPAModuleInfo()\n>  \t\tret = -EINVAL;\n>  \t}\n>  \n> +\tif (ret)\n> +\t\tLOG(IPAModule, Error)\n> +\t\t\t<< \"Error loading IPA module at \"\n> +\t\t\t<< libPath_;\n> +\n> +\n>  unmap:\n>  \tmunmap(map, soSize);\n>  close:","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 D232E67B6E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 Jun 2019 11:42:08 +0200 (CEST)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:a03f:44f0:8500:ca05:8177:199c:fed4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5A05533B;\n\tThu,  6 Jun 2019 11:42:08 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559814128;\n\tbh=OM+G6Ho1EOcYrA4+wZI7xXmAIvqitKCeNM5BrbE4d0A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=d8UXIxciDD4pcR5gUZS8LrzS4e+WaXgf4KMxkENJ/9vMjph0UChAdxn7oLA+/mEo7\n\tZz15DZTFpXCE1HmZiHBu++nXfUDE+BkWVtarVJ+ujzGgMJoDnuHxjszjkrzhM7WR/c\n\tRLtTFS8sXKeCfnIpVrWXfCRkbZakyatjvm/wqfOg=","Date":"Thu, 6 Jun 2019 12:41:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190606094154.GC12825@pendragon.ideasonboard.com>","References":"<20190605221817.966-1-paul.elder@ideasonboard.com>\n\t<20190605221817.966-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190605221817.966-4-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC PATCH 03/10] libcamera: ipa_module: add\n\tloading error messages","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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, 06 Jun 2019 09:42:09 -0000"}}]