On Tue, May 16, 2017 at 7:31 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> + snprintf(output_path, sizeof(output_path), "reindex_hash.sql");
>>
>> This looks suspiciously pointless. The contents of output_path will
>> always be precisely "reindex_hash.sql"; you could just char
>> *output_path = "reindex_hash.sql" and get the same effect.
>
> Sure, but the same code pattern is used in all other similar functions
> in version.c, refer new_9_0_populate_pg_largeobject_metadata. Both
> the ways will serve the purpose, do you think it makes sense to write
> this differently?
Yes. It's silly to copy a constant string into a new buffer just for
the heck of it. Perhaps the old instances should also be cleaned up
at some point, but even if we don't bother, copying absolutely
pointless coding into new places isn't getting us anywhere.
> Hmm, "./" is required for non-windows, but as mentioned above, this is
> not required for our case.
OK.
> I will send an updated patch once we agree on above points.
Sounds good.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
