Repository: syncthing/syncthing
File: lib/nat/service.go (Go)

Code before:
			s.mappings[i] = s.mappings[last]
			s.mappings[last] = nil
			s.mappings = s.mappings[:last]
			return
		}
	}
}

// updateMapping compares the addresses of the existing mapping versus the natds
// discovered, and removes any addresses of natds that do not exist, or tries to
// acquire mappings for natds which the mapping was unaware of before.
// Optionally takes renew flag which indicates whether or not we should renew
// mappings with existing natds
func (s *Service) updateMapping(ctx context.Context, mapping *Mapping, nats map[string]Device, renew bool) {
	renewalTime := time.Duration(s.cfg.Options().NATRenewalM) * time.Minute
	mapping.expires = time.Now().Add(renewalTime)

	change := false
	func() {
		mapping.mut.Lock()
		defer mapping.mut.Unlock()

		change = s.verifyExistingMappings(ctx, mapping, nats, renew)
		add := s.acquireNewMappings(ctx, mapping, nats)
		change = change || add
	}()

	if change {
		mapping.notify()
	}
}

func (s *Service) verifyExistingMappings(ctx context.Context, mapping *Mapping, nats map[string]Device, renew bool) (change bool) {
	leaseTime := time.Duration(s.cfg.Options().NATLeaseM) * time.Minute

	for id, address := range mapping.extAddresses {
		select {
		case <-ctx.Done():
			return false
		default:
		}

		// Delete addresses for NATDevice's that do not exist anymore
		nat, ok := nats[id]
		if !ok {
			mapping.removeAddress(id)
			change = true
			continue
		} else if renew {
			// Only perform renewals on the nat's that have the right local IP
			// address

Review comment (suggestion):
The intention here is to be able to `defer` the lock - or is there more to this function? Otherwise that seems a bit unnecessary to mean, compared to lock-unlock unclosing just 2-3 "plan" lines (no control flow shenanigans).

Code after:
	for i, existing := range s.mappings {
		if existing == mapping {
			mapping.clearAddresses()
			last := len(s.mappings) - 1
			s.mappings[i] = s.mappings[last]
			s.mappings[last] = nil
			s.mappings = s.mappings[:last]
			return
		}
	}
}

// updateMapping compares the addresses of the existing mapping versus the natds
// discovered, and removes any addresses of natds that do not exist, or tries to
// acquire mappings for natds which the mapping was unaware of before.
// Optionally takes renew flag which indicates whether or not we should renew
// mappings with existing natds
func (s *Service) updateMapping(ctx context.Context, mapping *Mapping, nats map[string]Device, renew bool) {
	renewalTime := time.Duration(s.cfg.Options().NATRenewalM) * time.Minute

	mapping.mut.Lock()

	mapping.expires = time.Now().Add(renewalTime)
	change := s.verifyExistingLocked(ctx, mapping, nats, renew)
	add := s.acquireNewLocked(ctx, mapping, nats)

	mapping.mut.Unlock()

	if change || add {
		mapping.notify()
	}
}

func (s *Service) verifyExistingLocked(ctx context.Context, mapping *Mapping, nats map[string]Device, renew bool) (change bool) {
	leaseTime := time.Duration(s.cfg.Options().NATLeaseM) * time.Minute

	for id, address := range mapping.extAddresses {
		select {
		case <-ctx.Done():
			return false
		default:
		}

		// Delete addresses for NATDevice's that do not exist anymore
		nat, ok := nats[id]
		if !ok {
			mapping.removeAddressLocked(id)
			change = true
			continue
		} else if renew {
			// Only perform renewals on the nat's that have the right local IP