On 2022/09/20 20:43, Bharath Rupireddy wrote:
- if (strlen(backupidstr) > MAXPGPATH) + if (state->name_overflowed == true) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("backup label too long (max %d bytes)", It does not strike me as a huge issue to force a truncation of such backup label names. 1024 is large enough for basically all users, in my opinion. Or you could just truncate in the internal logic, but still complain at MAXPGPATH - 1 as the last byte would be for the zero termination. In short, there is no need to complicate things with name_overflowed.We currently allow MAXPGPATH bytes of label name excluding null termination. I don't want to change this behaviour. In the attached v9 patch, I'm truncating the larger names to MAXPGPATH + 1 bytes in backup state (one extra byte for representing that the name has overflown, and another extra byte for null termination).
This looks much complicated to me. Instead of making allocate_backup_state() or reset_backup_state() store the label name in BackupState before do_pg_backup_start(), how about making do_pg_backup_start() do that after checking its length? Seems this can simplify the code very much. If so, ISTM that we can replace allocate_backup_state() and reset_backup_state() with just palloc0() and MemSet(), respectively. Also we can remove fill_backup_label_name(). Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
