Re: [PATCH] replace: forbid replacing an object with one of a different type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Christian Couder" <chriscool@xxxxxxxxxxxxx>
Sent: Wednesday, August 07, 2013 5:42 AM
Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

The doc will be updated in a later patch.

Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
---
If this patch is considered useful, I will update the doc and
maybe add tests.

Acked-by: Philip Oakley <philipoakley@xxxxxxx>
Improved documentation would always be useful.


builtin/replace.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..0246ab3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref,
const char *replace_ref,
   int force)
{
 unsigned char object[20], prev[20], repl[20];
+ enum object_type obj_type, repl_type;
 char ref[PATH_MAX];
 struct ref_lock *lock;

@@ -100,6 +101,14 @@ static int replace_object(const char *object_ref,
const char *replace_ref,
 if (check_refname_format(ref, 0))
 die("'%s' is not a valid ref name.", ref);

+ obj_type = sha1_object_info(object, NULL);
+ repl_type = sha1_object_info(repl, NULL);

Do (very) large blobs have any issues here? As I understand it, such
blobs, as with other smaller objects, need to be expanded to determine
the type. Is there a heuristic (and is it worth it) to do a 'large
object == blob' initial check to avoid such an expansion? Small objects
shouldn't be an overhead.

Just a thought.

+ if (obj_type != repl_type)
+ die("Object ref '%s' is of type '%s'\n"
+     "while replace ref '%s' is of type '%s'.",
+     object_ref, typename(obj_type),
+     replace_ref, typename(repl_type));

Perhaps start with "Objects must be of the same type.\n"

+
 if (read_ref(ref, prev))
 hashclr(prev);
 else if (!force)
--
1.8.4.rc1.22.g132b1a0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]