more code review fixes and use ruby builtins instead of grep/sed magi…

…c for port name

library/network/src/modules/PortAliases.rb

@@ -38,6 +38,8 @@
 
 module Yast
   class PortAliasesClass < Module
+    include Yast::Logger
+
     def main
       textdomain "base"
 
@@ -175,34 +177,35 @@ def LoadAndReturnPortToName(port_number)
     # Internal function for loading unknown ports into memory and returning them as integer
     def LoadAndReturnNameToPort(port_name)
       if !IsAllowedPortName(port_name)
-        Builtins.y2error("Disallwed port-name '%1'", port_name)
+        Builtins.y2error("Disallowed port-name '%1'", port_name)
         return nil
       end
 
-      grep_regexp = "^#{port_name}[ \\t]".shellescape
-      command = "/usr/bin/grep --perl-regexp #{grep_regexp} /etc/services " \
-        "| /usr/bin/sed \"s/[^ \\t]*[ \\t]*\\([^/ \\t]*\\).*/\\1/\""
-      found = Convert.to_map(SCR.Execute(path(".target.bash_output"), command))
-      alias_found = nil
+      path = File.join(WFM.scr_root, "etc/services")
+      content = ::File.read(path)
+      # find line with given port_name. Line looks like:
+      #   http               80/tcp       # World Wide Web HTTP
+      matching_lines = content.lines.grep(/^#{Regexp.escape(port_name)}\s/)
+      if matching_lines.empty?
+        log.error "service name not found. File.content #{content}"
+        return nil
+      end
 
-      if found["exit"] == 0
-        found["stdout"].split("\n").each do |alias_|
-          next if alias_.empty?
-          alias_found = Builtins.tointeger(alias_)
-        end
-      else
-        Builtins.y2error(
-          "Services Command: %1 -> %2",
-          command,
-          Ops.get_string(found, "stderr", "")
-        )
+      # lets use the first found entry, as often there are two entries for tcp and udp
+      port = matching_lines.first[/^\S+\s+(\d+)/, 1]
+      if port.nil?
+        log.error "wrong line in /etc/services: #{matching_lines.first}"
         return nil
       end
 
+      alias_found = port.to_i
+
       # store results for later requests
       Ops.set(@SERVICE_NAME_TO_PORT, port_name, alias_found)
 
       alias_found
+    rescue Errno, IOError => e
+      log.error "failed to read /etc/services #{e.inspect}"
     end
 
     # Function returns list of aliases (port-names and port-numbers) for

library/network/test/port_aliases_test.rb

@@ -0,0 +1,53 @@
+#!/usr/bin/env rspec
+
+require_relative "test_helper"
+
+Yast.import "PortAliases"
+
+FILE_CONTENT = <<EOS
+blocks             10288/tcp    # Blocks  [Carl_Malamud]
+blocks             10288/udp    # Blocks  [Carl_Malamud]
+cosir              10321/tcp    # Computer Op System Information Report  [Kevin_C_Barber]
+#                  10321/udp    Reserved 
+bngsync            10439/udp    # BalanceNG session table synchronization protocol  [Inlab_Software_GmbH] [Thomas_G._Obermair]
+#                  10439/tcp    Reserved 
+#                  10500/tcp    Reserved 
+hip-nat-t          10500/udp    # HIP NAT-Traversal  [RFC5770] [Ari_Keranen]
+MOS-lower          10540/tcp    # MOS Media Object Metadata Port  [Eric_Thorniley]
+MOS-lower          10540/udp    # MOS Media Object Metadata Port  [Eric_Thorniley]
+MOS-upper          10541/tcp    # MOS Running Order Port  [Eric_Thorniley]
+MOS-upper          10541/udp    # MOS Running Order Port  [Eric_Thorniley]
+EOS
+
+describe Yast::PortAliases do
+  describe ".LoadAndReturnNameToPort" do
+    before do
+      allow(::File).to receive(:read).and_return(FILE_CONTENT)
+    end
+
+    it "returns integer for given port name" do
+      expect(Yast::PortAliases.LoadAndReturnNameToPort("blocks")).to eq 10288
+    end
+
+    it "returns nil if given port name not found" do
+      expect(Yast::PortAliases.LoadAndReturnNameToPort("hellrouting")).to eq nil
+    end
+
+    it "does not interpret regexp characters" do
+      expect(Yast::PortAliases.LoadAndReturnNameToPort("b.*s")).to eq nil
+    end
+
+    it "respect SCR chroot" do
+      allow(Yast::WFM).to receive(:scr_root).and_return("/mnt")
+      expect(::File).to receive(:read).with("/mnt/etc/services").and_return(FILE_CONTENT)
+
+      Yast::PortAliases.LoadAndReturnNameToPort("test")
+    end
+
+    it "returns nil in case of IO Error" do
+      allow(::File).to receive(:read).and_raise(IOError)
+
+      expect(Yast::PortAliases.LoadAndReturnNameToPort("b.*s")).to eq nil
+    end
+  end
+end

library/packages/src/modules/PackageSystem.rb

@@ -327,8 +327,8 @@ def InitRPMQueryBinary
       )
         @_rpm_query_binary = "/usr/bin/rpmqpack "
         # than rpm itself
-      elsif Ops.greater_than(SCR.Read(path(".target.size"), "/bin/rpm"), -1)
-        @_rpm_query_binary = "/bin/rpm -q "
+      elsif Ops.greater_than(SCR.Read(path(".target.size"), "/usr/bin/rpm"), -1)
+        @_rpm_query_binary = "/usr/bin/rpm -q "
       end
 
       @_rpm_query_binary_initialized = true