[ 
https://issues.apache.org/jira/browse/TS-3960?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15009439#comment-15009439
 ] 

ASF GitHub Bot commented on TS-3960:
------------------------------------

Github user jpeach commented on the pull request:

    https://github.com/apache/trafficserver/pull/301#issuecomment-157497990
  
    - Remove _strlen and _memcpy usage (see TS-4029).
    
    - In ConfigFileGroup(), use ats_strdup() rather than malloc+memcpy.
    
    - In Rollback::createPathStr(), use the Layout::relative_to() that returns 
an allocated string.
    
    - Layout::relative_to, correctly deals with the file argument being
      an absolute path. So in Rollback::createPathStr(), you don't need
      to treat absolute paths specially.
    
    - Rollback::isFileNameRooted should be a function, not a member variable.
    
              bool isFileNameRooted() const {
                return *fileName == '/';
              }
    
    - ConfigFileGroup should not be responsible for unmarshalling data.
      The data marshalling format is a contract internal to the local
      manager. ConfigFileGroup should just deal with paths.
    
    - Why did you need to introduce ConfigFileGroup? An alternative
      would be to organize Rollback objects in a tree. Then you don't
      need to do all the name lookups; you can just keep a pointer from
      child to parent.
    
    - You can simplify the local manager protocol a lot. Rename
      MGMT_SIGNAL_CONFIG_FILE_GROUP_UPDATE to MGMT_SIGNAL_CONFIG_FILE_CHILD.
      The payload to this message is the parent config name and the file
      file path, like this:
    
            struct {
              int options;
              const char * parent; // config name
              const char * child;  // child path (absolute)
            };
    
      I'm not sure whether passing options is needed, but consider whether
      Rollback versioning should make backup copies of SSL keys. Passing
      options would allow you to deal with this.
    
      You don't need to marshall this by hand, use the API in MgmtMarshall.h
      to marshall the fields to a memory buffer. This marshalling should
      be done in a helper method on ProcessManager() that has an API
      similar to:
    
            ProcessManager::signalConfigFileChild(/* int options? */ const char 
* name, const char * path)
    
    - This adds a binary dependency from SSL configuration on the local
      manager that didn't exist before. Obviously I am OK with this, but
      I wanted to make that explicit.
    
    - Please sit with Brian and Thomas and make a TSQA test for this
      (in a separate pull request is OK).



> traffic_line -x doesn't reload SSL certs content
> ------------------------------------------------
>
>                 Key: TS-3960
>                 URL: https://issues.apache.org/jira/browse/TS-3960
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: SSL
>            Reporter: Zhang Zizhong
>            Assignee: Brian Geffon
>             Fix For: 6.1.0
>
>
> traffic_line -x doesn't reload  when SSL certs change file contents without 
> changing the file names.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to