Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.3.1
    • Component/s: Cleanup
    • Labels:
      None

      Description

      in RecCore.*

      int RecGetRecordInt(const char *name, RecInt * rec_int, bool lock = true);
      //-------------------------------------------------------------------------
      // RecGetRecordXXX
      //-------------------------------------------------------------------------
      int
      RecGetRecordInt(const char *name, RecInt *rec_int, bool lock)
      {
        int err;
        RecData data;
        if ((err = RecGetRecord_Xmalloc(name, RECD_INT, &data, lock)) == REC_ERR_OKAY)
          *rec_int = data.rec_int;
        return err;
      }
      

      and there is something heavy used:

      //-------------------------------------------------------------------------
      // Backwards Compatibility Items (REC_ prefix)
      //-------------------------------------------------------------------------
      #define REC_ReadConfigInt32(_var,_config_var_name) do { \
        RecInt tmp = 0; \
        RecGetRecordInt(_config_var_name, (RecInt*) &tmp); \
        _var = (int32_t)tmp; \
      } while (0)
      
      #define REC_ReadConfigInteger(_var,_config_var_name) do { \
        RecInt tmp = 0; \
        RecGetRecordInt(_config_var_name, &tmp); \
        _var = tmp; \
      } while (0)
      

      and a real case, the REC_ReadConfigInteger is renamed to IOCORE_ReadConfigInteger:

      RecInt cache_config_threads_per_disk = 12;
      #define IOCORE_ReadConfigInteger            REC_ReadConfigInteger
        IOCORE_ReadConfigInteger(cache_config_threads_per_disk, "proxy.config.cache.threads_per_disk");
      

      my question is, why it is so complex in all these renaming? why not just:

      RecGetRecordInt("proxy.config.cache.threads_per_disk", &cache_config_threads_per_disk);
      

      brief talk with Leif, we may need to cleanup the use of REC_*. make it a small task here

        Issue Links

          Activity

          Zhao Yongming created issue -
          Leif Hedstrom made changes -
          Field Original Value New Value
          Fix Version/s 3.1.2 [ 12317605 ]
          Affects Version/s 3.1.2 [ 12317605 ]
          Zhao Yongming made changes -
          Link This issue is duplicated by TS-1019 [ TS-1019 ]
          Leif Hedstrom made changes -
          Fix Version/s 3.1.3 [ 12317969 ]
          Fix Version/s 3.1.2 [ 12317605 ]
          Hide
          Leif Hedstrom added a comment -

          Moved to 3.1.4, please move bugs back to 3.1.3, which you will work on in the next 2 weeks.

          Show
          Leif Hedstrom added a comment - Moved to 3.1.4, please move bugs back to 3.1.3, which you will work on in the next 2 weeks.
          Leif Hedstrom made changes -
          Fix Version/s 3.1.4 [ 12318543 ]
          Fix Version/s 3.1.3 [ 12317969 ]
          Leif Hedstrom made changes -
          Fix Version/s 3.1.5 [ 12320056 ]
          Fix Version/s 3.1.4 [ 12318543 ]
          Hide
          Leif Hedstrom added a comment -

          Moving this out to 3.3.0, please move back to 3.1.4 if this will be worked on soon. Also, take a look at what can be closed here please!

          Show
          Leif Hedstrom added a comment - Moving this out to 3.3.0, please move back to 3.1.4 if this will be worked on soon . Also, take a look at what can be closed here please!
          Leif Hedstrom made changes -
          Fix Version/s 3.3.0 [ 12316495 ]
          Fix Version/s 3.1.5 [ 12320056 ]
          Leif Hedstrom made changes -
          Fix Version/s 3.3.1 [ 12321686 ]
          Fix Version/s 3.3.0 [ 12316495 ]
          Yunkai Zhang made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Yunkai Zhang added a comment -

          [PATCH 1/2] RecCore: refine P_RecCore.i and rename it to
          P_RecCore.cc

          The suffix of P_RecCore.i is weird for us:
          1) cscope can't find the function definition in it by default.
          2) using *.i suffix usually indicates that there are something bad in design. We can share the code gracefully if spliting the function/class carefully.

          Let's refine the code so that we can rename it to P_RecCore.cc safely.
          BTW, remove an useless file: ./iocore/utils/diags.i.

          I have noticed that there are several test_*.i files which is not so important for us, just keep it as it is.

          [PATCH 2/2] RecCore: remove unnecessary IOCORE_* wrapper on RecCore API

          For TS-977. Call RecCore API directly is mush better and clear than IOCORE_* wrapper.

          Show
          Yunkai Zhang added a comment - [PATCH 1/2] RecCore: refine P_RecCore.i and rename it to P_RecCore.cc The suffix of P_RecCore.i is weird for us: 1) cscope can't find the function definition in it by default. 2) using *.i suffix usually indicates that there are something bad in design. We can share the code gracefully if spliting the function/class carefully. Let's refine the code so that we can rename it to P_RecCore.cc safely. BTW, remove an useless file: ./iocore/utils/diags.i. I have noticed that there are several test_*.i files which is not so important for us, just keep it as it is. [PATCH 2/2] RecCore: remove unnecessary IOCORE_* wrapper on RecCore API For TS-977 . Call RecCore API directly is mush better and clear than IOCORE_* wrapper.
          Yunkai Zhang made changes -
          Attachment 0001-RecCore-refine-P_RecCore.i-and-rename-it-to-P_RecCor.patch [ 12564171 ]
          Attachment 0002-RecCore-remove-unnecessary-IOCORE_-wrapper-on-RecCor.patch [ 12564172 ]
          Yunkai Zhang made changes -
          Attachment 0002-RecCore-remove-unnecessary-IOCORE_-wrapper-on-RecCor.patch [ 12564172 ]
          Hide
          Yunkai Zhang added a comment -

          rebase it to upstream

          Show
          Yunkai Zhang added a comment - rebase it to upstream
          Yunkai Zhang made changes -
          Hide
          James Peach added a comment -

          Getting rid of these unnecessary macros is a nice improvement.

          Show
          James Peach added a comment - Getting rid of these unnecessary macros is a nice improvement.
          Hide
          Yunkai Zhang added a comment -

          Fix RecMessageSend regression caused by 0001-RecCore-refine-P_RecCore.i-and-rename-it-to-P_RecCor.patch.

          Show
          Yunkai Zhang added a comment - Fix RecMessageSend regression caused by 0001-RecCore-refine-P_RecCore.i-and-rename-it-to-P_RecCor.patch.
          Yunkai Zhang made changes -
          Hide
          James Peach added a comment -

          ming_zym: can you please commit these patches?

          Show
          James Peach added a comment - ming_zym: can you please commit these patches?
          Leif Hedstrom made changes -
          Fix Version/s 3.3.2 [ 12321745 ]
          Fix Version/s 3.3.1 [ 12321686 ]
          Hide
          Leif Hedstrom added a comment -

          Moving out to 3.3.2, since it's not critical.

          Show
          Leif Hedstrom added a comment - Moving out to 3.3.2, since it's not critical.
          Zhao Yongming made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 3.3.1 [ 12321686 ]
          Fix Version/s 3.3.2 [ 12321745 ]
          Resolution Fixed [ 1 ]
          Leif Hedstrom made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Zhao Yongming
              Reporter:
              Zhao Yongming
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development