Store location during Ascii input parsing for use in error messages

src/input/Manager.cc

@@ -1101,7 +1101,15 @@ Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType*
 					{
 					// Since we're building a (list) value for indexing into
 					// a table, it is for sure an error to miss a value.
-					Warning(i, "Skipping input with missing non-optional value");
+					auto source = i->reader->Info().source;
+					auto file_pos = vals[position]->GetFileLineNumber();
+
+					const char* warning = "Skipping input with missing non-optional value";
+					if ( source && file_pos != -1 )
+						Warning(i, "%s:%d: %s", source, file_pos, warning);
+					else
+						Warning(i, "%s", warning);
+
 					have_error = true;
 					}
 
@@ -1950,7 +1958,15 @@ RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const* v
 		else if ( ! vals[*position]->present &&
 		          ! request_type->FieldDecl(i)->GetAttr(zeek::detail::ATTR_OPTIONAL) )
 			{
-			Warning(stream, "Skipping input with missing non-optional value");
+			auto source = stream->reader->Info().source;
+			auto file_pos = vals[*position]->GetFileLineNumber();
+
+			const char* warning = "Skipping input with missing non-optional value";
+			if ( source && file_pos != -1 )
+				Warning(stream, "%s:%d: %s", source, file_pos, warning);
+			else
+				Warning(stream, "%s", warning);
+
 			have_error = true;
 			}
 		else

src/input/readers/ascii/Ascii.cc

@@ -157,8 +157,11 @@ bool Ascii::OpenFile()
 		return ! fail_on_file_problem;
 		}
 
-	read_location = std::make_unique<zeek::detail::Location>();
-	read_location->filename = util::copy_string(fname.c_str());
+	if ( ! read_location )
+		{
+		read_location = LocationPtr(new zeek::detail::Location());
+		read_location->filename = util::copy_string(fname.c_str());
+		}
 
 	StopWarningSuppression();
 	return true;
@@ -389,6 +392,8 @@ bool Ascii::DoUpdate()
 				{
 				// add non-present field
 				fields[fpos] = new Value((*fit).type, false);
+				if ( read_location )
+					fields[fpos]->SetFileLineNumber(read_location->first_line);
 				fpos++;
 				continue;
 				}
@@ -420,7 +425,6 @@ bool Ascii::DoUpdate()
 
 			Value* val = formatter->ParseValue(stringfields[(*fit).position], (*fit).name,
 			                                   (*fit).type, (*fit).subtype);
-
 			if ( ! val )
 				{
 				Warning(Fmt("Could not convert line '%s' of %s to Val. Ignoring line.",
@@ -429,6 +433,9 @@ bool Ascii::DoUpdate()
 				break;
 				}
 
+			if ( read_location )
+				val->SetFileLineNumber(read_location->first_line);
+
 			if ( (*fit).secondary_position != -1 )
 				{
 				// we have a port definition :)

src/input/readers/ascii/Ascii.h

@@ -91,7 +91,21 @@ private:
 	std::string path_prefix;
 
 	std::unique_ptr<threading::Formatter> formatter;
-	std::unique_ptr<zeek::detail::Location> read_location;
+
+	// zeek::detail::Location doesn't have a destructor because it's constexpr, so we have to
+	// define a custom deleter for the unique_ptr here to make sure the filename gets deleted
+	// correctly when the unique_ptr gets reset.
+	struct LocationDeleter
+		{
+		void operator()(zeek::detail::Location* loc) const
+			{
+			delete[] loc->filename;
+			delete loc;
+			}
+		};
+
+	using LocationPtr = std::unique_ptr<zeek::detail::Location, LocationDeleter>;
+	LocationPtr read_location;
 	};
 
 	} // namespace zeek::input::reader::detail

src/threading/Formatter.h

@@ -62,7 +62,7 @@ public:
 	 *
 	 * @param val the Value to render to the ODesc object.
 	 *
-	 * @param The name of a field associated with the value.
+	 * @param name The name of a field associated with the value.
 	 *
 	 * @return Returns true on success, false on error. Errors are also
 	 * flagged via the thread.
@@ -75,7 +75,7 @@ public:
 	 *
 	 * @param s The string to parse.
 	 *
-	 * @param The name of a field associated with the value. Used only
+	 * @param name The name of a field associated with the value. Used only
 	 * for error reporting.
 	 *
 	 * @return The new value, or null on error. Errors must also be

src/threading/SerialTypes.h

@@ -253,9 +253,16 @@ struct Value
 	static Val* ValueToVal(const std::string& source, const threading::Value* val,
 	                       bool& have_error);
 
+	void SetFileLineNumber(int line) { line_number = line; }
+	int GetFileLineNumber() const { return line_number; }
+
 private:
 	friend class IPAddr;
 	Value(const Value& other) = delete;
+
+	// For values read by the input framework, this can represent the line number
+	// containing this value. Used by the Ascii reader primarily.
+	int line_number = -1;
 	};
 
 	} // namespace zeek::threading

src/threading/formatters/Ascii.h

@@ -47,13 +47,13 @@ public:
 	 * separators.
 	 */
 	Ascii(MsgThread* t, const SeparatorInfo& info);
-	virtual ~Ascii();
+	~Ascii() override;
 
-	virtual bool Describe(ODesc* desc, Value* val, const std::string& name = "") const;
+	virtual bool Describe(ODesc* desc, Value* val, const std::string& name = "") const override;
 	virtual bool Describe(ODesc* desc, int num_fields, const Field* const* fields,
-	                      Value** vals) const;
+	                      Value** vals) const override;
 	virtual Value* ParseValue(const std::string& s, const std::string& name, TypeTag type,
-	                          TypeTag subtype = TYPE_ERROR) const;
+	                          TypeTag subtype = TYPE_ERROR) const override;
 
 private:
 	bool CheckNumberError(const char* start, const char* end, bool nonneg_only = false) const;

testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline

@@ -1,9 +1,9 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-warning: Skipping input with missing non-optional value
+warning: ../input.log:3: Skipping input with missing non-optional value
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Invalid value for subnet: 127.0.0.1
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Error while reading set or vector
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Could not convert line 'name	127.0.0.1' of ../input.log to Val. Ignoring line.
-warning: Skipping input with missing non-optional value
+warning: ../input.log:3: Skipping input with missing non-optional value
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Invalid value for subnet: 127.0.0.1
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Error while reading set or vector
 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Could not convert line 'name	127.0.0.1' of ../input.log to Val. Ignoring line.

testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out

@@ -1,13 +1,13 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 ### NOTE: This file has been sorted with diff-sort.
 
+EventErrorEvent, ../input.log:3: Skipping input with missing non-optional value, Reporter::WARNING
 EventErrorEvent, Could not convert line 'name\x09127.0.0.1' of ../input.log to Val. Ignoring line., Reporter::WARNING
 EventErrorEvent, Error while reading set or vector, Reporter::WARNING
 EventErrorEvent, Invalid value for subnet: 127.0.0.1, Reporter::WARNING
-EventErrorEvent, Skipping input with missing non-optional value, Reporter::WARNING
+TableErrorEvent, ../input.log:3: Skipping input with missing non-optional value, Reporter::WARNING
 TableErrorEvent, Could not convert line 'name\x09127.0.0.1' of ../input.log to Val. Ignoring line., Reporter::WARNING
 TableErrorEvent, Error while reading set or vector, Reporter::WARNING
 TableErrorEvent, Invalid value for subnet: 127.0.0.1, Reporter::WARNING
-TableErrorEvent, Skipping input with missing non-optional value, Reporter::WARNING
 {
 }