Dmitry Ivankov wrote: > In fast-import pack_id is 16-bit with MAX_PACK_ID reserved to identify > pre-existing objects. It is unlikely to wrap under reasonable settings > but still things in fast-import will break once it happens. > > Add a check and immediate die() as the simplest reaction to being unable > to continue the import. Makes a lot of sense. A few possible minor clarity improvements: - missing commas after "In fast-import" and before "with MAX_PACK_ID reserved" - "pre-existing objects": it would be clearer to say something like "objects this fast-import process instance did not write out to a packfile", like the comment before gfi_unpack_entry() does - I suppose "under reasonable settings" means "with a reasonable max-pack-size setting"? - "things will break" is a bit vague. - "immediate" -> "immediately" Maybe: In fast-import, pack_id is a 16-bit unsigned integer, with MAX_PACK_ID (2^16 - 1) reserved for use by objects that are not in a packfile that this fast-import process instance wrote. It is unusual for pack_id to hit MAX_PACK_ID with a reasonable --max-pack-size setting, but when it does, the pack_id stored in each "struct object_entry" wraps and fast-import gets utterly confused. Add a check and immediately die() so the operator can at least see what went wrong instead of experiencing an unexplained broken import. With or without that clarification, Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> Thanks! A test would still be nice, if someone has time to write one. -- 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