{"action":"created","issue":{"url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/953","repository_url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware","labels_url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/953/labels{/name}","comments_url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/953/comments","events_url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/953/events","html_url":"https://github.com/ARM-software/arm-trusted-firmware/pull/953","id":231500815,"number":953,"title":"Tegra misra fixes v1","user":{"login":"vwadekar","id":3041253,"avatar_url":"https://avatars1.githubusercontent.com/u/3041253?v=3","gravatar_id":"","url":"https://api.github.com/users/vwadekar","html_url":"https://github.com/vwadekar","followers_url":"https://api.github.com/users/vwadekar/followers","following_url":"https://api.github.com/users/vwadekar/following{/other_user}","gists_url":"https://api.github.com/users/vwadekar/gists{/gist_id}","starred_url":"https://api.github.com/users/vwadekar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vwadekar/subscriptions","organizations_url":"https://api.github.com/users/vwadekar/orgs","repos_url":"https://api.github.com/users/vwadekar/repos","events_url":"https://api.github.com/users/vwadekar/events{/privacy}","received_events_url":"https://api.github.com/users/vwadekar/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":28,"created_at":"2017-05-26T01:15:21Z","updated_at":"2017-06-08T13:37:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/pulls/953","html_url":"https://github.com/ARM-software/arm-trusted-firmware/pull/953","diff_url":"https://github.com/ARM-software/arm-trusted-firmware/pull/953.diff","patch_url":"https://github.com/ARM-software/arm-trusted-firmware/pull/953.patch"},"body":"Fixes for the defects found by the MISRA scanner in the Tegra port"},"comment":{"url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/comments/307105647","html_url":"https://github.com/ARM-software/arm-trusted-firmware/pull/953#issuecomment-307105647","issue_url":"https://api.github.com/repos/ARM-software/arm-trusted-firmware/issues/953","id":307105647,"user":{"login":"vwadekar","id":3041253,"avatar_url":"https://avatars1.githubusercontent.com/u/3041253?v=3","gravatar_id":"","url":"https://api.github.com/users/vwadekar","html_url":"https://github.com/vwadekar","followers_url":"https://api.github.com/users/vwadekar/followers","following_url":"https://api.github.com/users/vwadekar/following{/other_user}","gists_url":"https://api.github.com/users/vwadekar/gists{/gist_id}","starred_url":"https://api.github.com/users/vwadekar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vwadekar/subscriptions","organizations_url":"https://api.github.com/users/vwadekar/orgs","repos_url":"https://api.github.com/users/vwadekar/repos","events_url":"https://api.github.com/users/vwadekar/events{/privacy}","received_events_url":"https://api.github.com/users/vwadekar/received_events","type":"User","site_admin":false},"created_at":"2017-06-08T13:37:31Z","updated_at":"2017-06-08T13:37:31Z","body":"Thanks Dan. Will update the PR\n\nOn Thu, Jun 8, 2017, 3:41 AM danh-arm <notifications@github.com> wrote:\n\n> *@danh-arm* requested changes on this pull request.\n> ------------------------------\n>\n> In common/runtime_svc.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120840021>\n> :\n>\n> > @@ -36,7 +36,7 @@ uintptr_t handle_runtime_svc(uint32_t smc_fid,\n>  \t\t\t     unsigned int flags)\n>  {\n>  \tu_register_t x1, x2, x3, x4;\n> -\tint index, idx;\n> +\tuint32_t index, idx;\n>\n> No, index at least must remain signed since a negative index in the array\n> has a special meaning.\n>\n> The idx >= 0 check in line 44 is not needed if idx becomes unsigned.\n>\n> Since RT_SVC_DECS_NUM is unsigned, the 2nd use of index in line 47 should\n> be casted to an unsigned int.\n> ------------------------------\n>\n> In common/runtime_svc.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120842895>\n> :\n>\n> > @@ -36,7 +36,7 @@ uintptr_t handle_runtime_svc(uint32_t smc_fid,\n>  \t\t\t     unsigned int flags)\n>\n> This patch should come before any changes to the signed/unsigned\n> definitions (e.g. include: add U()/ULL() macros for constants), to\n> demonstrate signed/unsigned cleanness in the existing codebase.\n> ------------------------------\n>\n> In common/runtime_svc.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120843941>\n> :\n>\n> > @@ -89,7 +89,8 @@ static int32_t validate_rt_svc_desc(const rt_svc_desc_t *desc)\n>   ******************************************************************************/\n>  void runtime_svc_init(void)\n>  {\n> -\tint rc = 0, index, start_idx, end_idx;\n> +\tint rc = 0, start_idx, end_idx;\n> +\tuint index;\n>\n> uint is not a type we use. Use unsigned int. It looks like start_idx and\n> end_idx should be unsigned too.\n> ------------------------------\n>\n> In lib/psci/psci_common.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120846878>\n> :\n>\n> > @@ -332,7 +332,7 @@ void psci_get_parent_pwr_domain_nodes(unsigned int cpu_idx,\n>  \t\t\t\t      unsigned int node_index[])\n>  {\n>  \tunsigned int parent_node = psci_cpu_pd_nodes[cpu_idx].parent_node;\n> -\tint i;\n> +\tuint32_t i;\n>\n> minor: Prefer not to use a mix of uint32_t and unsigned int for\n> parent_node and i. The TF coding guidelines suggest using the latter,\n> although I know that MISRA prefers the former.\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120848732>\n> :\n>\n> > @@ -203,9 +203,9 @@ static void xlat_tables_unmap_region(xlat_ctx_t *ctx, mmap_region_t *mm,\n>  \t\t\t\t     const uintptr_t table_base_va,\n>  \t\t\t\t     uint64_t *const table_base,\n>  \t\t\t\t     const int table_entries,\n> -\t\t\t\t     const int level)\n> +\t\t\t\t     const uint level)\n>\n> unsigned int\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120849013>\n> :\n>\n> >  {\n> -\tassert(level >= ctx->base_level && level <= XLAT_TABLE_LEVEL_MAX);\n> +\tassert(level >= (uint)ctx->base_level && level <= XLAT_TABLE_LEVEL_MAX);\n>\n> It looks like xlat_ctx_t.ctx should be unsigned int rather than casting\n> each time it is used. Possibly other fields in that struct too.\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120849146>\n> :\n>\n> > @@ -323,7 +323,7 @@ static void xlat_tables_unmap_region(xlat_ctx_t *ctx, mmap_region_t *mm,\n>  \t\t\tbreak;\n>  \t}\n>\n> -\tif (level > ctx->base_level)\n> +\tif (level > (uint)ctx->base_level)\n>\n> It looks like xlat_ctx_t.ctx should be unsigned int\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120849199>\n> :\n>\n> > @@ -468,9 +468,9 @@ static uintptr_t xlat_tables_map_region(xlat_ctx_t *ctx, mmap_region_t *mm,\n>  \t\t\t\t   const uintptr_t table_base_va,\n>  \t\t\t\t   uint64_t *const table_base,\n>  \t\t\t\t   const int table_entries,\n> -\t\t\t\t   const int level)\n> +\t\t\t\t   const uint level)\n>\n> unsigned int\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120849225>\n> :\n>\n> >  {\n> -\tassert(level >= ctx->base_level && level <= XLAT_TABLE_LEVEL_MAX);\n> +\tassert(level >= (uint)ctx->base_level && level <= XLAT_TABLE_LEVEL_MAX);\n>\n> It looks like xlat_ctx_t.ctx should be unsigned int\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120849250>\n> :\n>\n> > @@ -497,7 +497,7 @@ static uintptr_t xlat_tables_map_region(xlat_ctx_t *ctx, mmap_region_t *mm,\n>  \t}\n>\n>  #if PLAT_XLAT_TABLES_DYNAMIC\n> -\tif (level > ctx->base_level)\n> +\tif (level > (uint)ctx->base_level)\n>\n> It looks like xlat_ctx_t.ctx should be unsigned int\n> ------------------------------\n>\n> In lib/xlat_tables_v2/xlat_tables_internal.c\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120850454>\n> :\n>\n> > @@ -1060,7 +1060,7 @@ void init_xlation_table(xlat_ctx_t *ctx)\n>  #if PLAT_XLAT_TABLES_DYNAMIC\n>  \t\tctx->tables_mapped_regions[j] = 0;\n>  #endif\n> -\t\tfor (int i = 0; i < XLAT_TABLE_ENTRIES; i++)\n> +\t\tfor (uint32_t i = 0; i < XLAT_TABLE_ENTRIES; i++)\n>\n> It looks odd that this becomes unsigned but the other loop counters in\n> this function remain signed. It looks like they should also become unsigned.\n> ------------------------------\n>\n> In include/lib/aarch64/arch.h\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#discussion_r120852369>\n> :\n>\n> > @@ -278,48 +278,48 @@\n>  #define ADDR_MASK_36_TO_39\t0x000000F000000000UL\n>  #define ADDR_MASK_32_TO_35\t0x0000000F00000000UL\n>\n> Looks like these ones should be using ULL() macro\n>\n> —\n> You are receiving this because you were mentioned.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/ARM-software/arm-trusted-firmware/pull/953#pullrequestreview-42835892>,\n> or mute the thread\n> <https://github.com/notifications/unsubscribe-auth/AC5n5Qq7Q-zNpyFew6d7iTkInHwt7Fd1ks5sB8_kgaJpZM4NnEQE>\n> .\n>\n"}}
{"actor":{"display_login":"vwadekar"}}