{"thread":{"id":"28266","subject":"[PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()","startedAt":"2011-08-31T04:48:46Z","lastAt":"2011-09-02T07:09:23Z","messageCount":4,"participants":["Tay Ray Chuan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174596","messageId":"1314766126-5060-1-git-send-email-rctay89@gmail.com","threadId":"28266","inReplyTo":null,"subject":"[PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-08-31T04:48:46Z","receivedAt":"2011-08-31T04:48:46Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe\neven if xdl_init_classifier() isn't called. This may occur in the case\nwhere diff is run with --histogram and a call to, say, xdl_prepare_ctx()\nfails.\n\nSigned-off-by: Tay Ray Chuan \n---\n\nThis should go into 'rc/histogram-diff' in 'next'.\n\n xdiff/xprepare.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex 620fc9a..4323596 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -239,6 +239,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n \tlong enl1, enl2, sample;\n \txdlclassifier_t cf;\n \n+\tcf.rchash = NULL;\n+\tcf.ncha.head = NULL;\n+\n \t/*\n \t * For histogram diff, we can afford a smaller sample size and\n \t * thus a poorer estimate of the number of lines, as the hash\n-- \n1.7.6.1.706.gaa5cf\n"},{"id":"174632","messageId":"7vei01zf89.fsf@alter.siamese.dyndns.org","threadId":"28266","inReplyTo":"1314766126-5060-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-31T17:02:30Z","receivedAt":"2011-08-31T17:02:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe\n> even if xdl_init_classifier() isn't called. This may occur in the case\n> where diff is run with --histogram and a call to, say, xdl_prepare_ctx()\n> fails.\n>\n> Signed-off-by: Tay Ray Chuan \n\nThanks. Did you find this by code inspection?\n\n> xdiff/xprepare.c | 3 +++\n> 1 files changed, 3 insertions(+), 0 deletions(-)\n>\n> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\n> index 620fc9a..4323596 100644\n> --- a/xdiff/xprepare.c\n> +++ b/xdiff/xprepare.c\n> @@ -239,6 +239,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n> \tlong enl1, enl2, sample;\n> \txdlclassifier_t cf;\n> \n> +\tcf.rchash = NULL;\n> +\tcf.ncha.head = NULL;\n\nWould it be more appropriate to use memcpy(&cf, 0, sizeof(cf)) instead, so\nthat we wouldn't have to worry about a similar breakage when a new field\nis added to \"struct xdlclassifier\" later?\n"},{"id":"174633","messageId":"7vaaapzf6f.fsf@alter.siamese.dyndns.org","threadId":"28266","inReplyTo":"7vei01zf89.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-31T17:03:36Z","receivedAt":"2011-08-31T17:03:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Tay Ray Chuan writes:\n>\n>> Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe\n>> even if xdl_init_classifier() isn't called. This may occur in the case\n>> where diff is run with --histogram and a call to, say, xdl_prepare_ctx()\n>> fails.\n>>\n>> Signed-off-by: Tay Ray Chuan \n>\n> Thanks. Did you find this by code inspection?\n>\n>> xdiff/xprepare.c | 3 +++\n>> 1 files changed, 3 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\n>> index 620fc9a..4323596 100644\n>> --- a/xdiff/xprepare.c\n>> +++ b/xdiff/xprepare.c\n>> @@ -239,6 +239,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n>> \tlong enl1, enl2, sample;\n>> \txdlclassifier_t cf;\n>> \n>> +\tcf.rchash = NULL;\n>> +\tcf.ncha.head = NULL;\n>\n> Would it be more appropriate to use memcpy(&cf, 0, sizeof(cf)) instead, so\n\nOops, I meant memset(), obviously.\n\n> that we wouldn't have to worry about a similar breakage when a new field\n> is added to \"struct xdlclassifier\" later?\n"},{"id":"174735","messageId":"1314947363-4596-1-git-send-email-rctay89@gmail.com","threadId":"28266","inReplyTo":"7vaaapzf6f.fsf@alter.siamese.dyndns.org","subject":"[PATCH] xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-09-02T07:09:23Z","receivedAt":"2011-09-02T07:09:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe\neven if xdl_init_classifier() isn't called, which is the case when diff\nis run with --histogram.\n\nSigned-off-by: Tay Ray Chuan \n\n---\n\nOn Thu, Sep 1, 2011 at 1:02 AM, Junio C Hamano wrote:\n> Tay Ray Chuan writes:\n>\n>> Ensure that the xdl_free_classifier() call on xdlclassifier_t cf is safe\n>> even if xdl_init_classifier() isn't called. This may occur in the case\n>> where diff is run with --histogram and a call to, say, xdl_prepare_ctx()\n>> fails.\n>>\n>> Signed-off-by: Tay Ray Chuan \n>\n> Thanks. Did you find this by code inspection?\n\nIf by \"code inspection\" you meant eye-balling, then yes. :)\n\nOn Thu, Sep 1, 2011 at 1:03 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> [snip]\n>>> @@ -239,6 +239,9 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n>>>      long enl1, enl2, sample;\n>>>      xdlclassifier_t cf;\n>>>\n>>> +    cf.rchash = NULL;\n>>> +    cf.ncha.head = NULL;\n>>\n>> Would it be more appropriate to use memcpy(&cf, 0, sizeof(cf)) instead, so\n>\n> Oops, I meant memset(), obviously.\n\nRight, thanks.\n---\n xdiff/xprepare.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex 620fc9a..5b676b7 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -239,6 +239,8 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n \tlong enl1, enl2, sample;\n \txdlclassifier_t cf;\n \n+\tmemset(&cf, 0, sizeof(cf));\n+\n \t/*\n \t * For histogram diff, we can afford a smaller sample size and\n \t * thus a poorer estimate of the number of lines, as the hash\n-- \n1.7.6.1.706.gaa5cf\n"}]}