Convention around encode/decode versions

[Yixin Jin <yjin77@xxxxxxxx>]

Hi folks,

I noticed that the encode/decode functions enforce versions in order to achieve backward compatibility and provide a upgrade path forward. However, I'd like to confirm the standard of practice around the use of versions in this case. If decode() function states the compatv is 9, there should be no code inside that handles the case of struct_v < 9, since this condition should never be satisfied. Is this the right understanding? I saw this block of code in RGWUserInfo::decode():

  void decode(bufferlist::const_iterator& bl) {
       DECODE_START_LEGACY_COMPAT_LEN_32(22, 9, 9, bl);                                                                                
       if (struct_v >= 2) {                                                                                                            
         uint64_t old_auid;
         decode(old_auid, bl);                                                                                                         
       }
       std::string access_key;                                                                                                         
       std::string secret_key;                                                                                                         
      decode(access_key, bl);                                                                                                          
      decode(secret_key, bl);                                                                                                          
      if (struct_v < 6) {                                                                                                              
        RGWAccessKey k;
        k.id = access_key;
        k.key = secret_key;
        access_keys[access_key] = k;                                                                                                   
      }

I don't see why we need to handle the case of struct_v < 6 when compatv is 9. Is it safe to assume that this if statement is a dead code? If so, could we also assume that the following if block in its encode() function should be removed, too?

  void encode(bufferlist& bl) const {
       ENCODE_START(22, 9, bl);
       encode((uint64_t)0, bl); // old auid
       std::string access_key;
       std::string secret_key;
       if (!access_keys.empty()) {
         std::map<std::string, RGWAccessKey>::const_iterator iter = access_keys.begin();
         const RGWAccessKey& k = iter->second;
         access_key = k.id;
         secret_key = k.key;
       }

Thanks,

Yixin

_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to dev-leave@xxxxxxx