{"thread":{"id":"36280","subject":"[PATCH v3 1/2] fsck.c: modify fsck_ident() and fsck_commit()","startedAt":"2014-03-24T12:08:21Z","lastAt":"2014-03-25T06:50:47Z","messageCount":2,"participants":["Ashwin Jha","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"237440","messageId":"1395662901-10617-1-git-send-email-ajha.dev@gmail.com","threadId":"36280","inReplyTo":null,"subject":"[PATCH v3 1/2] fsck.c: modify fsck_ident() and fsck_commit()","fromName":"Ashwin Jha","fromEmail":"ajha.dev@gmail.com","sentAt":"2014-03-24T12:08:21Z","receivedAt":"2014-03-24T12:08:21Z","isPatch":true,"sender":{"key":"ajha.dev@gmail.com","avatar":null},"body":"In fsck_ident(): Replace argument char **ident with const char **ident\nIn fsck_commit(): Replace char *buffer with const char *buffer\n\nIn both the cases, referenced memory addresses are not modified. So, it\nwill be a good practice, to declare them as const.\n\nSigned-off-by: Ashwin Jha <ajha.dev@gmail.com>\n---\n\nChange in fsck_commit() and fsck_ident() as per the discussion with\nEric (follow at [1]).\n[1]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html\n\n fsck.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex 64bf279..b5f017f 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -243,7 +243,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \treturn retval;\n }\n \n-static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n+static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func)\n {\n \tchar *end;\n \n@@ -284,7 +284,7 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n \n static int fsck_commit(struct commit *commit, fsck_error error_func)\n {\n-\tchar *buffer = commit->buffer;\n+\tconst char *buffer = commit->buffer;\n \tunsigned char tree_sha1[20], sha1[20];\n \tstruct commit_graft *graft;\n \tint parents = 0;\n-- \n1.9.1\n"},{"id":"237529","messageId":"CAPig+cQg_GRwSpQb53OYHcu6=9B+9QhwBSCRasq5jN+-QHjoDg@mail.gmail.com","threadId":"36280","inReplyTo":"1395662901-10617-1-git-send-email-ajha.dev@gmail.com","subject":"Re: [PATCH v3 1/2] fsck.c: modify fsck_ident() and fsck_commit()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-25T06:50:47Z","receivedAt":"2014-03-25T06:50:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 24, 2014 at 8:08 AM, Ashwin Jha <ajha.dev@gmail.com> wrote:\n> Subject: fsck.c: modify fsck_ident() and fsck_commit()\n\nThe subject should summarize the change while being concise and\nexpressive. The above is a bit lacking. A better subject might be:\n\n    Subject: fsck: add missing 'const's\n\n> In fsck_ident(): Replace argument char **ident with const char **ident\n> In fsck_commit(): Replace char *buffer with const char *buffer\n\nNo need to repeat in prose what the patch itself already states more\nconcisely and precisely. You can drop this part.\n\n> In both the cases, referenced memory addresses are not modified. So, it\n> will be a good practice, to declare them as const.\n\nThe subject suggested above (\"add missing 'const's\") implies that the\nreferenced memory is never modified, so it's not really necessary to\nexplain it here too. Stating that it's good practice may be a\nreasonable way to justify the change, although is also rather obvious.\nI'd say that the suggested subject alone is a sufficient commit\nmessage for this patch, though you and/or Junio might feel otherwise.\n\n> Signed-off-by: Ashwin Jha <ajha.dev@gmail.com>\n> ---\n>\n> Change in fsck_commit() and fsck_ident() as per the discussion with\n> Eric (follow at [1]).\n> [1]: http://git.661346.n2.nabble.com/PATCH-Modify-fsck-commit-Replace-memcmp-by-skip-prefix-td7606321.html\n\nProviding a link to the previous attempt is indeed courteous to\nreviewers. Since reviewer time is precious (due to this being a\nhigh-volume mailing list), it is possible to be even more helpful by\nproviding more detail in your summary of changes. For instance,\ninstead of saying \"Change in ... as per discussion\", you might say:\n\n    Changes since v2 [1]:\n\n    Expanded to 2-patch series.\n\n    patch 1/1: add missing 'const's to fsck_ident() argument and\n    fsck_commit() 'buffer' variable so that 2/2 doesn't have to\n    cast-away constness from the result of skip_prefix().\n\n    patch 2/2: dropped the now unnecessary casts\n\nThe patch itself looks fine, thanks.\n\n>  fsck.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/fsck.c b/fsck.c\n> index 64bf279..b5f017f 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -243,7 +243,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n>         return retval;\n>  }\n>\n> -static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n> +static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func)\n>  {\n>         char *end;\n>\n> @@ -284,7 +284,7 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n>\n>  static int fsck_commit(struct commit *commit, fsck_error error_func)\n>  {\n> -       char *buffer = commit->buffer;\n> +       const char *buffer = commit->buffer;\n>         unsigned char tree_sha1[20], sha1[20];\n>         struct commit_graft *graft;\n>         int parents = 0;\n> --\n> 1.9.1\n"}]}